Hi, I finally managed to track the problem with keyboard events with OSWindow. However, I cannot fix it without introducing a crash in an autorelease, or a memory leak. The SDL2 problem can be fixed by applying the following patch: diff --git a/platforms/iOS/vm/Common/Classes/sqSqueakEventsAPI.m b/platforms/iOS/vm/Common/Classes/sqSqueakEventsAPI.m index 94c3c4c82..4b5aec8c5 100644 --- a/platforms/iOS/vm/Common/Classes/sqSqueakEventsAPI.m +++ b/platforms/iOS/vm/Common/Classes/sqSqueakEventsAPI.m @@ -100,9 +100,7 @@ sqInt ioProcessEvents(void) { // We need to run the vmIOProcessEvents even if we are using SDL2, // otherwise we end with some double free errors in an autorelease pool. - vmIOProcessEvents(); - if(ioProcessEventsHandler && ioProcessEventsHandler != vmIOProcessEvents) - ioProcessEventsHandler(); + ioProcessEventsHandler(); However, this introduces a crash in an Objective-C autorelease pool, in at leas two locations: diff --git a/platforms/iOS/vm/OSX/sqSqueakOSXApplication+events.m b/platforms/iOS/vm/OSX/sqSqueakOSXApplication+events.m index 4f2ac06ea..c1ec3e1ea 100644 --- a/platforms/iOS/vm/OSX/sqSqueakOSXApplication+events.m +++ b/platforms/iOS/vm/OSX/sqSqueakOSXApplication+events.m @@ -152,9 +152,10 @@ - (void) recordCharEvent:(NSString *) unicodeString fromView: (NSView <sqSqueakO } } - NSString *lookupString = AUTORELEASEOBJ([[NSString alloc] initWithCharacters: &unicode length: 1]); + NSString *lookupString = [[NSString alloc] initWithCharacters: &unicode length: 1]; [lookupString getBytes: &macRomanCharacter maxLength: 1 usedLength: NULL encoding: NSMacOSRomanStringEncoding options: 0 range: picker remainingRange: NULL]; + [lookupString release]; evt.pressCode = EventKeyDown; unsigned short keyCodeRemembered = evt.charCode; diff --git a/platforms/iOS/vm/OSX/sqSqueakOSXOpenGLView.m b/platforms/iOS/vm/OSX/sqSqueakOSXOpenGLView.m index c441332a3..72aa8c42d 100644 --- a/platforms/iOS/vm/OSX/sqSqueakOSXOpenGLView.m +++ b/platforms/iOS/vm/OSX/sqSqueakOSXOpenGLView.m @@ -747,7 +747,7 @@ - (NSString *) dealWithOpenStepChars: (NSString *) openStep { -(void)keyDown:(NSEvent*)theEvent { - keyBoardStrokeDetails *aKeyBoardStrokeDetails = AUTORELEASEOBJ([[keyBoardStrokeDetails alloc] init]); + keyBoardStrokeDetails *aKeyBoardStrokeDetails = ([[keyBoardStrokeDetails alloc] init]); aKeyBoardStrokeDetails.keyCode = [theEvent keyCode]; aKeyBoardStrokeDetails.modifierFlags = [theEvent modifierFlags]; @@ -757,6 +757,8 @@ -(void)keyDown:(NSEvent*)theEvent { [self interpretKeyEvents: down]; self.lastSeenKeyBoardStrokeDetails = NULL; } + + //[aKeyBoardStrokeDetails release]; } In the last location I had to comment my attempt on doing a manual release because it introduces a crash. OSWindow can be tested in a fresh Pharo image by doing the following in a workspace: FFIExternalStructure allSubclassesDo: #rebuildFieldAccessors.
SDL_Event initialize. SDL2 findSDL2. OSWindowWorldMorph new open.
I have attached the full diff with my bug fix attempt. I hope that someone that is more familiar with the OS X platform code and Objective-C can remove these autoreleases without introducing crashes. When executed in a debugger, the crash is detected in SDL2 Cocoa_PumpEvents: void
Cocoa_PumpEvents(_THIS)
{ @autoreleasepool
{
/* Update activity every 30 seconds to prevent screensaver */
SDL_VideoData *data = (SDL_VideoData *)_this->driverdata;
if (_this->suspend_screensaver && !data->screensaver_use_iopm) {
Uint32 now = SDL_GetTicks();
if (!data->screensaver_activity ||
SDL_TICKS_PASSED(now, data->screensaver_activity + 30000)) {
UpdateSystemActivity(UsrActivity);
data->screensaver_activity = now;
}
}
for ( ; ; ) {
NSEvent *event = [NSApp nextEventMatchingMask:NSAnyEventMask
untilDate:[NSDate distantPast] inMode:NSDefaultRunLoopMode dequeue:YES ];
if ( event == nil ) {
break;
}
if (!s_bShouldHandleEventsInSDLApplication) {
Cocoa_DispatchEvent(event);
}
// Pass events down to SDLApplication to be handled in sendEvent:
[NSApp sendEvent:event];
}
}}
For tracking these actual calls to #autorelease, I had to set a breakpoint on the #autorelease Objective-C selector, and log the stacktrace to find where it is called in our code. Best regards, Ronie
Thanks ronie! This is important for the future of Pharo. On Thu, Nov 2, 2017 at 2:48 AM, Ronie Salgado <roniesalg@gmail.com> wrote:
Hi,
I finally managed to track the problem with keyboard events with OSWindow. However, I cannot fix it without introducing a crash in an autorelease, or a memory leak.
The SDL2 problem can be fixed by applying the following patch:
diff --git a/platforms/iOS/vm/Common/Classes/sqSqueakEventsAPI.m b/platforms/iOS/vm/Common/Classes/sqSqueakEventsAPI.m index 94c3c4c82..4b5aec8c5 100644 --- a/platforms/iOS/vm/Common/Classes/sqSqueakEventsAPI.m +++ b/platforms/iOS/vm/Common/Classes/sqSqueakEventsAPI.m @@ -100,9 +100,7 @@ sqInt ioProcessEvents(void) {
// We need to run the vmIOProcessEvents even if we are using SDL2, // otherwise we end with some double free errors in an autorelease pool. - vmIOProcessEvents(); - if(ioProcessEventsHandler && ioProcessEventsHandler != vmIOProcessEvents) - ioProcessEventsHandler(); + ioProcessEventsHandler();
However, this introduces a crash in an Objective-C autorelease pool, in at leas two locations:
diff --git a/platforms/iOS/vm/OSX/sqSqueakOSXApplication+events.m b/platforms/iOS/vm/OSX/sqSqueakOSXApplication+events.m index 4f2ac06ea..c1ec3e1ea 100644 --- a/platforms/iOS/vm/OSX/sqSqueakOSXApplication+events.m +++ b/platforms/iOS/vm/OSX/sqSqueakOSXApplication+events.m @@ -152,9 +152,10 @@ - (void) recordCharEvent:(NSString *) unicodeString fromView: (NSView <sqSqueakO } }
- NSString *lookupString = AUTORELEASEOBJ([[NSString alloc] initWithCharacters: &unicode length: 1]); + NSString *lookupString = [[NSString alloc] initWithCharacters: &unicode length: 1]; [lookupString getBytes: &macRomanCharacter maxLength: 1 usedLength: NULL encoding: NSMacOSRomanStringEncoding options: 0 range: picker remainingRange: NULL]; + [lookupString release];
evt.pressCode = EventKeyDown; unsigned short keyCodeRemembered = evt.charCode;
diff --git a/platforms/iOS/vm/OSX/sqSqueakOSXOpenGLView.m b/platforms/iOS/vm/OSX/sqSqueakOSXOpenGLView.m index c441332a3..72aa8c42d 100644 --- a/platforms/iOS/vm/OSX/sqSqueakOSXOpenGLView.m +++ b/platforms/iOS/vm/OSX/sqSqueakOSXOpenGLView.m @@ -747,7 +747,7 @@ - (NSString *) dealWithOpenStepChars: (NSString *) openStep {
-(void)keyDown:(NSEvent*)theEvent { - keyBoardStrokeDetails *aKeyBoardStrokeDetails = AUTORELEASEOBJ([[keyBoardStrokeDetails alloc] init]); + keyBoardStrokeDetails *aKeyBoardStrokeDetails = ([[keyBoardStrokeDetails alloc] init]); aKeyBoardStrokeDetails.keyCode = [theEvent keyCode]; aKeyBoardStrokeDetails.modifierFlags = [theEvent modifierFlags];
@@ -757,6 +757,8 @@ -(void)keyDown:(NSEvent*)theEvent { [self interpretKeyEvents: down]; self.lastSeenKeyBoardStrokeDetails = NULL; } + + //[aKeyBoardStrokeDetails release]; }
In the last location I had to comment my attempt on doing a manual release because it introduces a crash.
OSWindow can be tested in a fresh Pharo image by doing the following in a workspace:
FFIExternalStructure allSubclassesDo: #rebuildFieldAccessors. SDL_Event initialize. SDL2 findSDL2. OSWindowWorldMorph new open.
I have attached the full diff with my bug fix attempt. I hope that someone that is more familiar with the OS X platform code and Objective-C can remove these autoreleases without introducing crashes.
When executed in a debugger, the crash is detected in SDL2 Cocoa_PumpEvents:
void
Cocoa_PumpEvents(_THIS)
{ @autoreleasepool
{
/* Update activity every 30 seconds to prevent screensaver */
SDL_VideoData *data = (SDL_VideoData *)_this->driverdata;
if (_this->suspend_screensaver && !data->screensaver_use_iopm) {
Uint32 now = SDL_GetTicks();
if (!data->screensaver_activity ||
SDL_TICKS_PASSED(now, data->screensaver_activity + 30000)) {
UpdateSystemActivity(UsrActivity);
data->screensaver_activity = now;
}
}
for ( ; ; ) {
NSEvent *event = [NSApp nextEventMatchingMask:NSAnyEventMask untilDate:[NSDate distantPast] inMode:NSDefaultRunLoopMode dequeue:YES ];
if ( event == nil ) {
break;
}
if (!s_bShouldHandleEventsInSDLApplication) {
Cocoa_DispatchEvent(event);
}
// Pass events down to SDLApplication to be handled in sendEvent:
[NSApp sendEvent:event];
}
}}
For tracking these actual calls to #autorelease, I had to set a breakpoint on the #autorelease Objective-C selector, and log the stacktrace to find where it is called in our code.
Best regards, Ronie
participants (2)
-
Ronie Salgado -
Stephane Ducasse