Skip to content

fix: disown boxed wrappers freed via *_free/*_unref methods (#429)#432

Merged
romgrk merged 1 commit into
masterfrom
fix/429-disown-on-free
Jun 14, 2026
Merged

fix: disown boxed wrappers freed via *_free/*_unref methods (#429)#432
romgrk merged 1 commit into
masterfrom
fix/429-disown-on-free

Conversation

@romgrk

@romgrk romgrk commented Jun 14, 2026

Copy link
Copy Markdown
Owner

Summary

Core fix for #429. A boxed/struct wrapper owns its backing memory and frees it on GC (gated by Boxed::owns_memory). When user code calls an introspected method that frees the instance itself — e.g. GLib.MainLoop's unref(), following the normal C idiom — node-gtk still thought it owned the memory and freed it again at GC. That double-free is a benign g_main_loop_unref: ref_count > 0 warning on lenient allocators, but a hard SIGSEGV during node's loop teardown when gi.startLoop() is active on libffi 3.5 (Ubuntu 26).

This is the general counterpart to the example fix in #431.

Fix

After an instance method whose C symbol ends in _free/_unref on a struct/union/boxed container, disown the wrapper (src/function.cc → new DisownBoxed in src/boxed.cc):

  • clear owns_memory so the GC finalizer won't free it again;
  • null the data pointer so later use fails cleanly rather than as a use-after-free.

Scoped deliberately:

  • boxed/struct/union only — GObject lifetime is managed separately, and gtk_widget_destroy() & co. don't free the wrapper (hence _free/_unref only, not _destroy).
  • Worst case is a leak, not a crash — if someone unref()s a multi-ref object without dropping the last ref, the wrapper simply won't unref at GC. Mixing manual refcounting with node-gtk's GC ownership was already unsupported.

Verified on Ubuntu 26.04 (rootless podman; libffi 3.5.2 / gir 1.86.0 / glib 2.88 / node 22)

The crashing pattern — gi.startLoop() + new GLib.MainLoop() + explicit loop.unref() + natural exit:

crashes
before 30/30
with this fix 0/30

Plus a new regression test tests/conversion__boxed_explicit_unref.js (passes), and the full suite stays green locally (76 passing; only the pre-existing env require.js).

Note

Leaves the broader question (should ref/unref/free be hidden entirely?) untouched — this makes the common, idiomatic case safe with a minimal, scoped change.

🤖 Generated with Claude Code

A boxed/struct wrapper owns its memory and frees it on GC. When user code
calls an introspected method that frees the instance itself (e.g.
GLib.MainLoop's unref(), following the C idiom), node-gtk still believed it
owned the memory and freed it again at GC — a double-free. Benign on lenient
allocators, but a hard SIGSEGV during node's loop teardown with gi.startLoop()
on libffi 3.5 (Ubuntu 26).

After a *_free/*_unref instance method on a struct/union/boxed, disown the
wrapper: clear owns_memory so the finalizer won't free it, and null the data
pointer. Worst case becomes a leak (e.g. unref() that didn't drop the last
ref) rather than a crash; mixing manual refcounting with node-gtk's GC
ownership was already unsupported.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@romgrk romgrk merged commit 69e9367 into master Jun 14, 2026
9 checks passed
romgrk added a commit that referenced this pull request Jun 14, 2026
Move the per-platform install/build, testing, and browser-demo
documentation out of the README into doc/installation.md, leaving a
short link in its place. Also record the #429 boxed double-free fix
(#432) in the v2.0.0 changelog.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant