[OpenSmalltalk/opensmalltalk-vm] callout from interp.c code to objc/swift code need autorelease pool (#374)

List overview All Threads
Download

newer

older

VM Maker:...

[OpenSmalltalk/opensmalltalk-vm]...

johnmci

28 Feb 2019 28 Feb '19

9:38 p.m.

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?

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/374

Attachments:

attachment.html (text/html — 2.1 KB)

Show replies by date

Tobias Pape

28 Feb 28 Feb

9:50 p.m.

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

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/374#issuecomment-46...

maenu

2 Mar 2 Mar

6:20 p.m.

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) ```

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/374#issuecomment-46...

John M McIntosh

4 Mar 4 Mar

8:46 a.m.

New subject: [OpenSmalltalk/opensmalltalk-vm] callout from interp.c code to objc/swift code need autorelease pool (#374)

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 notifications@github.com wrote:

...

Instead of the PR [#373](https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/373), maybe we should solve this issue. Below is the trace to the leak in [#373](https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/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)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, [view it on GitHub](https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/374#issuecomment-46...), or [mute the thread](https://github.com/notifications/unsubscribe-auth/AhLyW11A8LBgYYh3ih-GCmljcS...).

maenu

12 Mar 12 Mar

12:19 p.m.

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

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:

1. Is there any way to ensure a complete list of all callouts that need to be added to the autorelease pool? 2. 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? 3. 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?

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/374#issuecomment-47...

johnmci

13 Mar 13 Mar

6:25 p.m.

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

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/374#issuecomment-47...

Eliot Miranda

6:27 p.m.

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 ye 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 we invoke VM code I am all for that.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/374#issuecomment-47...

Tobias Pape

6:44 p.m.

This already happens: - https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/a8a1dc1e33267e0fa2dab... - https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/a8a1dc1e33267e0fa2dab...

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)

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/374#issuecomment-47...

johnmci

6:45 p.m.

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.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/374#issuecomment-47...

johnmci

6:52 p.m.

...

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







       @autoreleasepool {
[opensmalltalk-vm/platforms/iOS/vm/Common/Classes/sqSqueakMainApplication.m](https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/a8a1dc1e33267e0fa2dab...)
     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.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/374#issuecomment-47...

1894

Age (days ago)

1907

Last active (days ago)

vm-dev@lists.squeakfoundation.org

9 comments

5 participants

tags (0)

participants (5)

Eliot Miranda
John M McIntosh
johnmci
maenu
Tobias Pape