feat(gobject, closure): wrap GObject with GC-able C++ wrapper to fix closures reference loop#375
feat(gobject, closure): wrap GObject with GC-able C++ wrapper to fix closures reference loop#375peat-psuwit wants to merge 2 commits intoromgrk:masterfrom
Conversation
There was a problem hiding this comment.
This is a good approach and I'd be happy to merge it if we can make it work. This project hasn't received enough love lately, and I haven't done much work to keep it working with recent versions of nodejs. I'm fine with dropping older versions, but the CI will need attention to keep tests passing on at least one version. Let's see if we can get #374 merged first.
|
Looks like #374 could be merged soon.
I haven't followed V8 development lately, do you have a link to the changelog/docs for that? Do you expect any big issue with compat between <23 and 23+? |
I can't find doc, but the relevant commit is: nodejs/node@0be79f4 NodeJS actually gives an API for wrapping part (and that part technically have compatibility layer). However I can't find an equivalent API for unwrapping from NodeJS and has to directly use V8 APIs. I've already written code for that and put |
|
#374 was merged and I'll release it as v1.0.0 soon. Afaiu, this is all internal and wouldn't break the external API so it can be released as a minor/patch once it's ready. |
693b9a4 to
60fae0a
Compare
TODO: test on Node 23+. Node-GTK itself requires update. Has PR. Instead of storing pointer to GObject directly in JS Object, store a C++ GC-able wrapper class. This enables us to track JS memory embedded inside GObject, avoiding reference loop.
Replace a Persistent in Closure with a TracedReference. Then, keep track of created Closure in GObjectWrapper. When GObjectWrapper is traced by cppgc, trace those reference to tell cppgc they're still alive. By not creating a Persistent, we prevent a reference loop where the Persistent holds function, holds JS wrapper, holds GObject, holds Persistent. When TracedReference, evetually the whole loop will not be traced by cppgc, allowing the whole loop to be dropped. This approach is inspired by PyGObject.
60fae0a to
ec4c879
Compare
Instead of storing pointer to GObject directly in JS Object, store a
C++ GC-able wrapper class.
Replace a Persistent in Closure with a TracedReference. Then, keep track
of created Closure in GObjectWrapper. When GObjectWrapper is traced by
cppgc, trace those reference to tell cppgc they're still alive.
By not creating a Persistent, we prevent a reference loop where the
Persistent holds function, holds JS wrapper, holds GObject, holds
Persistent. When TracedReference, evetually the whole loop will not be
traced by cppgc, allowing the whole loop to be dropped.
This approach is inspired by PyGObject and utilize V8's Oilpan (also called cppgc), C++ garbage collector integrated with V8. This, however, means cutting support for NodeJS older than version 20.6. That said, NodeJS 18 has just became end-of-life (not counting commercial support), so maybe it's not that bad, compared to not leaking memory?
TODO:
gobject.cc(seeFIXMEin the code). Also consider movingGObjectWrapperclass declaration togobject.h.