progrium/darwinkit

integrate objc `release` with Go garbage collector

Closed this issue · 8 comments

as mentioned in #12, not using a pointer for type object means its passed as a value. i believe the suggestion was to use it as a pointer type so we could use SetFinalizer effectively. however, this brings up some questions.

since sendMsg creates (maybe all) object values, would we want to track those pointer values so there is only one object in go holding reference to an objc pointer?

would this negate the need for using autoreleasepool at all?

mgood commented

IIUC I do think we'll want to structure it so that there's only one Go value holding each objc reference. Though if multiple objc calls return references to the same object, each reference would be unique. So as each Go reference is GC-ed, it will release the objc, decrementing its reference count, and eventually freeing the object.

However, it doesn't sound like this removes the need for an autoreleasepool. Releasing the Go references would handle objects that are passed back to Go as a return value. However, an objc call may allocate additional objects that are added to the pool, but not directly referenced by the return value. So, calling release on the return value would not also release those values.

I'll follow up on #12 with some additional thoughts.

mgood commented

Hmm, so I think there's an issue here with how the package currently handles some primitive values. E.g. calls to Send() that return raw numeric values still wrap it in an Object, where the caller is expected to use .Int() to convert it. But for these, the wrapping Object instance doesn't know that the ptr value is really just a number and not a pointer. So calling Release() in the finalizer would crash.

So, it seems like this is first going to require some changes to how it wraps return values to distinguish which are wrapping pointers vs raw values.

Yea, there is some simplicity to just assuming objects, esp since most of the APIs are just returning objects. In a way, letting the caller decide to Release gets around this as well. I'm ok with the idea of saying all return values, whether values or objects, will be objects, but once it starts impacting any magic we do with releasing it makes things more complicated.

Also keep in mind we will have access to schema information for APIs, so when we create wrappers in Go we can decide based on the return value type to do one thing or another.

We could say that Go GC integration via finalizer is only done with wrapped/generated code and do it there and not at the objc package level.

mgood commented

We could say that Go GC integration via finalizer is only done with wrapped/generated code and do it there and not at the objc package level.

Yeah, that could be interesting, and also led me to thinking about a simpler way to just wrap the Object interface to add the GC functionality in #51.
We can leave the objc package a little lower-level and move the automated memory management into the wrapper layer with some of the helpers.

mgood commented

I've retitled this to better reflect the goal. A pointer is necessary for the GC so that we pass around a single instance in memory, though with an approach like #51 it's possible to add that GC support to a wrapper.

However, looking at this again, I do think we'd need to change how we handle Objective-C pointers to ensure that re-wrapping the object in a different type doesn't trigger a GC of the original.

For example you may need code like this to wrap some reference as an NSString:

b := NSString_fromRef(a)
// equivalent to:
// b := NSString_fromPointer(unsafe.Pointer(a.Pointer()))

If there are no further references to a, it could be garbage collected, triggering the finalizer even though b is expected to be an equivalent reference.

So, we'd want a different way of re-wrapping a "handle" that refers to an object without losing the original Go pointer that holds it.

mgood commented

@ lunixbochs I'm going to continue the discussion from this comment here. #50 (comment)

Note that a big reason to not use implicit autoreleasepool, is that it depends on LockOSThread() to work correctly, and a user can easily starve the go thread pool if they don't know that. If you warn them to use the autoreleasepool API prominently, you also have a chance to tell them about the caveats and manage expectations.

Yeah, my ideal about using autoreleasepools implicitly may be too idealistic, but I was kind of wondering if by combining that with the GC for releasing objects referenced on the Go side we might be able to handle most memory management needs automatically.

The thread pool is an interesting point, do you have some more info on when that might happen? I haven't tested this extensively, but I was re-reading the docs on SetMaxThreads to try to better understand the limitations:

A Go program creates a new thread only when a goroutine is ready to run but all the existing threads are blocked in system calls, cgo calls, or are locked to other goroutines due to use of runtime.LockOSThread.
https://pkg.go.dev/runtime/debug@go1.17.2#SetMaxThreads

Based on that it sounds like cgo is also locked to a single thread. Would calling LockOSThread() around a single cgo call (as would happen with the implicit autoreleasepool idea) have much additional impact if the cgo call would also be locked to the thread?

Implicitly creating autoreleasepools for every method call still have other issues, but do you have more info on how LockOSThread interacts here and if it presents other problems I'm missing?

Going to close this for now since it should all be covered now:
https://github.com/progrium/macdriver/blob/main/docs/memorymanagement.md

But we can open a new issue for anything else from here.