Re: OSWindow OS X VM bug

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Re: OSWindow OS X VM bug

Stephane Ducasse-3
Thanks ronie!
This is important for the future of Pharo.

On Thu, Nov 2, 2017 at 2:48 AM, Ronie Salgado <[hidden email]> 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
>