opensmalltalk-vm callout from interp.c code to objc/swift code need autorelease pool

original issue exposed in pumpRunLoopEventSendAndSignal but perhaps the autoreleasepool wrapper should be higher up in say ioProcessEvents()

what is happening is the call out is creating autorelease objects, but when do they get released.

Other call out should be audited for the same issue. Some routines might not need a wrapper?

Feb 28 '19 20:02 johnmci

We also have a "global" autorelease in the main app, that should catch most thing, but we might no get there "in time"…

Feb 28 '19 20:02 krono

Instead of the PR #373, maybe we should solve this issue. Below is the trace to the leak in #373, ioProcessEvents seems like an appropriate place for the autoreleasepool. How can we find other places that enter Objective-C and need a pool?

Direct leak of 219512 byte(s) in 1193 object(s) allocated from:
    #0 0x10bb1fffc in __sanitizer_mz_malloc (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x5dffc)
    #1 0x7fffdd224281 in malloc_zone_malloc (libsystem_malloc.dylib:x86_64+0x2281)
    #2 0x7fffc6de33d5 in AllocateAndInitializeEvent(__CFAllocator const*) (HIToolbox:x86_64+0x33d5)
    #3 0x7fffc6e3fc4d in CopyEventInternal(__CFAllocator const*, OpaqueEventRef*) (HIToolbox:x86_64+0x5fc4d)
    #4 0x7fffc6e49228 in CopyEventAs (HIToolbox:x86_64+0x69228)
    #5 0x7fffc6e1d349 in CreateEventWithCGEvent (HIToolbox:x86_64+0x3d349)
    #6 0x7fffc6e1b0e2 in CreateAndPostEventWithCGEvent(__CGEvent*, unsigned int, unsigned char, __CFMachPortBoost*) (HIToolbox:x86_64+0x3b0e2)
    #7 0x7fffc6e275cd in Convert1CGEvent(unsigned char) (HIToolbox:x86_64+0x475cd)
    #8 0x7fffc6e2745e in MainLoopObserver(unsigned int, OpaqueEventRef*, void*) (HIToolbox:x86_64+0x4745e)
    #9 0x7fffc6de8368 in _NotifyEventLoopObservers (HIToolbox:x86_64+0x8368)
    #10 0x7fffc6e10ea5 in RunCurrentEventLoopInMode (HIToolbox:x86_64+0x30ea5)
    #11 0x7fffc6e10bf8 in ReceiveNextEventCommon (HIToolbox:x86_64+0x30bf8)
    #12 0x7fffc6e10b25 in _BlockUntilNextEventMatchingListInModeWithFilter (HIToolbox:x86_64+0x30b25)
    #13 0x7fffc53a5a53 in _DPSNextEvent (AppKit:x86_64+0x46a53)
    #14 0x7fffc5b217ed in -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] (AppKit:x86_64+0x7c27ed)
    #15 0x10b7ce07c in -[sqSqueakOSXApplication(events) pumpRunLoopEventSendAndSignal:] sqSqueakOSXApplication+events.m:78
    #16 0x10b7ce7d9 in -[sqSqueakOSXApplication(events) pumpRunLoop] sqSqueakOSXApplication+events.m:105
    #17 0x10b7e4898 in vmIOProcessEvents sqSqueakEventsAPI.m:80
    #18 0x10b7e49c7 in ioProcessEvents sqSqueakEventsAPI.m:103
    #19 0x10b5b5653 in checkForEventsMayContextSwitch gcc3x-cointerp.c:62680
    #20 0x10b5c17ed in ceCheckForInterrupts gcc3x-cointerp.c:15188
    #21 0x119a75794  (<unknown module>)
    #22 0x10b566712 in interpret gcc3x-cointerp.c:2754
    #23 0x10b7ebada in -[sqSqueakMainApplication runSqueak] sqSqueakMainApplication.m:201
    #24 0x7fffc93786fc in __NSFirePerformWithOrder (Foundation:x86_64+0xd76fc)
    #25 0x7fffc78cfc56 in __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ (CoreFoundation:x86_64h+0xa6c56)
    #26 0x7fffc78cfbc6 in __CFRunLoopDoObservers (CoreFoundation:x86_64h+0xa6bc6)
    #27 0x7fffc78b05f8 in __CFRunLoopRun (CoreFoundation:x86_64h+0x875f8)
    #28 0x7fffc78b0033 in CFRunLoopRunSpecific (CoreFoundation:x86_64h+0x87033)
    #29 0x7fffc6e10ebb in RunCurrentEventLoopInMode (HIToolbox:x86_64+0x30ebb)

Mar 02 '19 17:03 maenu

Most of the callbacks to platform code are documented here

https://isqueak.org/PlatformVMAPI

.... John M. McIntosh. Corporate Smalltalk Consulting Ltd https://www.linkedin.com/in/smalltalk

Sent from ProtonMail Mobile

On Sat, Mar 2, 2019 at 09:20, maenu [email protected] wrote:

Instead of the PR #373, maybe we should solve this issue. Below is the trace to the leak in #373, ioProcessEvents seems like an appropriate place for the autoreleasepool. How can we find other places that enter Objective-C and need a pool?

Direct leak of 219512 byte(s) in 1193 object(s) allocated from: #0 0x10bb1fffc in __sanitizer_mz_malloc (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x5dffc) #1 0x7fffdd224281 in malloc_zone_malloc (libsystem_malloc.dylib:x86_64+0x2281) #2 0x7fffc6de33d5 in AllocateAndInitializeEvent(__CFAllocator const*) (HIToolbox:x86_64+0x33d5) #3 0x7fffc6e3fc4d in CopyEventInternal(__CFAllocator const*, OpaqueEventRef*) (HIToolbox:x86_64+0x5fc4d) #4 0x7fffc6e49228 in CopyEventAs (HIToolbox:x86_64+0x69228) #5 0x7fffc6e1d349 in CreateEventWithCGEvent (HIToolbox:x86_64+0x3d349) #6 0x7fffc6e1b0e2 in CreateAndPostEventWithCGEvent(__CGEvent*, unsigned int, unsigned char, __CFMachPortBoost*) (HIToolbox:x86_64+0x3b0e2) #7 0x7fffc6e275cd in Convert1CGEvent(unsigned char) (HIToolbox:x86_64+0x475cd) #8 0x7fffc6e2745e in MainLoopObserver(unsigned int, OpaqueEventRef*, void*) (HIToolbox:x86_64+0x4745e) #9 0x7fffc6de8368 in _NotifyEventLoopObservers (HIToolbox:x86_64+0x8368) #10 0x7fffc6e10ea5 in RunCurrentEventLoopInMode (HIToolbox:x86_64+0x30ea5) #11 0x7fffc6e10bf8 in ReceiveNextEventCommon (HIToolbox:x86_64+0x30bf8) #12 0x7fffc6e10b25 in _BlockUntilNextEventMatchingListInModeWithFilter (HIToolbox:x86_64+0x30b25) #13 0x7fffc53a5a53 in _DPSNextEvent (AppKit:x86_64+0x46a53) #14 0x7fffc5b217ed in -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] (AppKit:x86_64+0x7c27ed) #15 0x10b7ce07c in -[sqSqueakOSXApplication(events) pumpRunLoopEventSendAndSignal:] sqSqueakOSXApplication+events.m:78 #16 0x10b7ce7d9 in -[sqSqueakOSXApplication(events) pumpRunLoop] sqSqueakOSXApplication+events.m:105 #17 0x10b7e4898 in vmIOProcessEvents sqSqueakEventsAPI.m:80 #18 0x10b7e49c7 in ioProcessEvents sqSqueakEventsAPI.m:103 #19 0x10b5b5653 in checkForEventsMayContextSwitch gcc3x-cointerp.c:62680 #20 0x10b5c17ed in ceCheckForInterrupts gcc3x-cointerp.c:15188 #21 0x119a75794 () #22 0x10b566712 in interpret gcc3x-cointerp.c:2754 #23 0x10b7ebada in -[sqSqueakMainApplication runSqueak] sqSqueakMainApplication.m:201 #24 0x7fffc93786fc in __NSFirePerformWithOrder (Foundation:x86_64+0xd76fc) #25 0x7fffc78cfc56 in CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION (CoreFoundation:x86_64h+0xa6c56) #26 0x7fffc78cfbc6 in __CFRunLoopDoObservers (CoreFoundation:x86_64h+0xa6bc6) #27 0x7fffc78b05f8 in __CFRunLoopRun (CoreFoundation:x86_64h+0x875f8) #28 0x7fffc78b0033 in CFRunLoopRunSpecific (CoreFoundation:x86_64h+0x87033) #29 0x7fffc6e10ebb in RunCurrentEventLoopInMode (HIToolbox:x86_64+0x30ebb)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

Mar 04 '19 07:03 OpenSmalltalk-Bot

I took the list of VM APIs at https://isqueak.org/PlatformVMAPI and created a regex that searches for definitions of them:

\w+\s+\*?\s*\b(ioExit|ioDisablePowerManager|ioMicroMSecs|ioMSecs|ioLowResMSecs|ioRelinquishProcessorForMicroseconds|dir_Lookup|dir_Create|dir_Delete|dir_Delimitor|dir_SetMacFileTypeAndCreator|dir_GetMacFileTypeAndCreator|dir_PathToWorkingDir|sqFTruncate|getAttributeIntoLength|attributeSize|sqGetFilenameFromString|ioScreenSize|ioScreenDepth|ioHasDisplayDepth|ioForceDisplayUpdate|display_ioShowDisplay|ioProcessEvents|ioSetInputSemaphore|ioGetNextEvent|ioBeep|sqAllocateMemory|sqGrowMemoryBy|sqShrinkMemoryBy|sqMemoryExtraBytesLeft|reserveExtraCHeapBytes|imageName|getImageName|imageNamePutLength|imageNameGetLength|imageNameSize|vmPathSize|vmPathGetLength|sqImageFileOpen|insufficientMemorySpecifiedError|insufficientMemoryAvailableError|unableToReadImageError|browserPluginReturnIfNeeded|browserPluginInitialiseIfNeeded|ioFormPrint|ioSetFullScreen|ioSeconds|ioSetCursor|ioSetCursorWithMask|ioSetCursorARGB|ioSetDisplayMode|ioGetButtonState|ioCanRenameImage|secCanGetFileTypeOfSize|secHasSocketAccess|clearProfile|clipboardSize|clipboardReadIntoAt|clipboardWriteFromAt\n|ioLoadModule|ioFindExternalFunctionIn|ioFreeModule)\b

Here are the locations that define these methods in *.m files. I guess there are more methods that are not in the list.

A few questions remain for me:

Is there any way to ensure a complete list of all callouts that need to be added to the autorelease pool?
How does a general fix have to look? Wrap all definitions with an auto-release block, or is there a global machinery that can be hooked into?
Regarding ARC vs. non-ARC: Does @autoreleasepool even exist in non-ARC? If no, then there is no point of using AUTORELEASEOBJ() etc. If yes, how to audit the code for missing AUTORELEASEOBJ() calls?

Mar 12 '19 11:03 maenu

The issue is calling out to swift or OBJ-C code could make autoreleased object which exist both in ARC and non-arc.

At camp smalltalk I'll go thru the list and see which require wrapping. Some do not. In general the onus has been on the called method to do the right thing with memory management, so there is no automatic wrapping to cleanup autoreleased objects. I should benchmark the setup/takedown if it's really fast then maybe we can see how to make it explicit

Mar 13 '19 17:03 johnmci

But it is important to note that any wrapping can be done in the iOS code and should not be e.g. pushed further out to the VM code. Why isn't it possible to wrap the entire VM execution in an appropriate autorelease context in the iOS startup code before invoking the VM itself? iOS is "in charge" of the launch process. If the code needs to be refactored to invoke the VM through a function that can apply wrapping before VM code is invoked then I am all for the refactoring.

Mar 13 '19 17:03 eliotmiranda

This already happens:

https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/a8a1dc1e33267e0fa2dab22959e41d0a072420d9/platforms/iOS/vm/Common/main.m#L59
https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/a8a1dc1e33267e0fa2dab22959e41d0a072420d9/platforms/iOS/vm/Common/Classes/sqSqueakMainApplication.m#L154

But I think this is not enough. If I understand ARC correctly, leaving the autoreleasepool-ed region triggeres the cleanup etc. Hence, this would would only trigger after the respecive code leaves. Which is, not during normal operation…

(@johnmci please correct me if I'm wrong)

Mar 13 '19 17:03 krono

An example is.

ioSetCursorARGB in interp.c calls the C interface ioSetCursorARGB in sqSqueakCursorAPI.m where there is a call to execute the Obj-C methods in sqSqueakOSXApplication+cursor.m, the ioSetCursorARGB contain a autorelease pool wrapper.

However this is fine grained as the sqSqueakOSXApplication+cursor.m code could change and introduce a autorelease object before the guard clauses, etc.

This is what happened in pumpRunLoopEventSendAndSignal where there was no autorelease pool because the code didn't require it, but over the years a code change introduced the autorelease object leak.

So a safer choice would be to wrap ioSetCursorARGB in sqSqueakCursorAPI.m to ensure mistakes don't happen as the actual implementation code is changed.

In general I originally wrote the code to call from interp.c to an exposed C routine, to the obj-c logic, so I am hoping this change will be 'simple'.

There is more of an issue for plugins, as they are of course C routines, but could call Obj-C. For those it might just fall back to the authors to ensure they don't leak.

Mar 13 '19 17:03 johnmci

This already happens:
opensmalltalk-vm/platforms/iOS/vm/Common/main.m
     Line 59
  in
  [a8a1dc1](/OpenSmalltalk/opensmalltalk-vm/commit/a8a1dc1e33267e0fa2dab22959e41d0a072420d9)







       @autoreleasepool {
opensmalltalk-vm/platforms/iOS/vm/Common/Classes/sqSqueakMainApplication.m
     Line 154
  in
  [a8a1dc1](/OpenSmalltalk/opensmalltalk-vm/commit/a8a1dc1e33267e0fa2dab22959e41d0a072420d9)







       @autoreleasepool {
But I think this is not enough. If I understand ARC correctly, leaving the autoreleasepool-ed region triggeres the cleanup etc. Hence, this would would only trigger after the respecive code leaves. Which is, not during normal operation…

(@johnmci please correct me if I'm wrong)

In these cases the called routine only terminates when Squeak terminates. It's a programming style pattern, but doesn't of course cleanup memory as the interpreter is running.

In some variations of the VM (like for a browser plugin) we wrote the code so that after N byte codes interpreted, we would save the state, and exit the interpreter loop, and return back to the caller. Later the caller would call back into the interp.c and resume. At that point we could unwind the autorelease pool. However I don't recommend that as a solution due to overhead.

We have tools to check for memory leaking, just need to run them from time to time to ensure a os-x or iOS platform change hasn't introduced a leak by forgetting a auto-release pool.

Mar 13 '19 17:03 johnmci

opensmalltalk-vm opensmalltalk-vm copied to clipboard

callout from interp.c code to objc/swift code need autorelease pool

opensmalltalk-vm
opensmalltalk-vm copied to clipboard