Skip to content

Commit

Permalink
Fix intern.Box being freed before finalizers are called
Browse files Browse the repository at this point in the history
Related issue: #130
  • Loading branch information
diamondburned committed Feb 9, 2024
1 parent feb2025 commit a1bfe15
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 25 deletions.
2 changes: 1 addition & 1 deletion pkg/core/glib/connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func closureNew(v *Object, f interface{}) *C.GClosure {
closures := closure.RegistryType.Get(v.box)
closures.Register(unsafe.Pointer(gclosure), fs)

// C.g_object_watch_closure(v.native(), gclosure)
C.g_object_watch_closure(v.native(), gclosure)
C.g_closure_set_meta_marshal(gclosure, C.gpointer(v.Native()), (*[0]byte)(C._gotk4_goMarshal))
C.g_closure_add_finalize_notifier(gclosure, C.gpointer(v.Native()), (*[0]byte)(C._gotk4_removeClosure))

Expand Down
18 changes: 18 additions & 0 deletions pkg/core/intern/intern.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#include "intern.h"

const gchar *gotk4_object_type_name(gpointer obj) {
return G_OBJECT_TYPE_NAME(obj);
};

gboolean gotk4_intern_remove_toggle_ref(gpointer obj) {
// First, remove the toggle reference. This forces the object to be freed,
// calling any necessary finalizers.
g_object_remove_toggle_ref(G_OBJECT(obj), (GToggleNotify)goToggleNotify,
NULL);

// Only once the object is freed, we can remove it from the weak reference
// registry, since now the finalizers will not be called anymore.
goFinishRemovingToggleRef(obj);

return FALSE;
}
25 changes: 3 additions & 22 deletions pkg/core/intern/intern.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,7 @@
package intern

// #cgo pkg-config: gobject-2.0
// #include <glib-object.h>
//
// extern void goToggleNotify(gpointer, GObject*, gboolean);
// static const gchar* gotk4_object_type_name(gpointer obj) { return G_OBJECT_TYPE_NAME(obj); };
//
// gboolean gotk4_intern_remove_toggle_ref(gpointer obj) {
// g_object_remove_toggle_ref(G_OBJECT(obj), (GToggleNotify)goToggleNotify, NULL);
// }
// #include "intern.h"
import "C"

import (
Expand Down Expand Up @@ -313,20 +306,6 @@ func finalizeBox(dummy *boxDummy) {
toggleRefs.Println(objInfoRes, "finalizeBox: moving finalize to next GC cycle")
}
} else {
// If the closures are weak-referenced, then the object reference hasn't
// been toggled yet. Since the object is going away and we're still
// weakly referenced, we can wipe the closures away.
delete(shared.weak, dummy.gobject)

shared.mu.Unlock()

// Unreference the object. This will potentially free the object as
// well. The closures are definitely gone at this point.
// C.g_object_remove_toggle_ref(
// (*C.GObject)(unsafe.Pointer(dummy.gobject)),
// (*[0]byte)(C.goToggleNotify), nil,
// )

// Do this in the main loop instead. This is because finalizers are
// called in a finalizer thread, and our remove_toggle_ref might be
// destroying other main loop objects.
Expand All @@ -335,6 +314,8 @@ func finalizeBox(dummy *boxDummy) {
(*[0]byte)(C.gotk4_intern_remove_toggle_ref),
C.gpointer(dummy.gobject))

shared.mu.Unlock()

if toggleRefs != nil {
toggleRefs.Println(objInfoRes,
"finalizeBox: remove_toggle_ref queued for next main loop iteration")
Expand Down
6 changes: 6 additions & 0 deletions pkg/core/intern/intern.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include <glib-object.h>

extern void goToggleNotify(gpointer, GObject *, gboolean);
extern void goFinishRemovingToggleRef(gpointer);
const gchar *gotk4_object_type_name(gpointer obj);
gboolean gotk4_intern_remove_toggle_ref(gpointer obj);
41 changes: 39 additions & 2 deletions pkg/core/intern/intern_export.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package intern

// #cgo pkg-config: gobject-2.0
// #include <glib-object.h>
// #include "intern.h"
import "C"

import "unsafe"
import (
"log"
"unsafe"
)

// goToggleNotify is called by GLib on each toggle notification. It doesn't
// actually free anything and relies on Box's finalizer to free both the box and
Expand Down Expand Up @@ -32,3 +35,37 @@ func goToggleNotify(_ C.gpointer, obj *C.GObject, isLastInt C.gboolean) {
toggleRefs.Println(objInfo(unsafe.Pointer(obj)), "goToggleNotify: is last =", isLast)
}
}

// finishRemovingToggleRef is called after the toggle reference removal routine
// is dispatched in the main loop. It removes the GObject from the strong and
// weak global maps and unsets the finalizer.
//
//go:nosplit
//export goFinishRemovingToggleRef
func goFinishRemovingToggleRef(gobject unsafe.Pointer) {
shared.mu.Lock()
defer shared.mu.Unlock()

box, strong := gets(gobject)
if box == nil {
// Extremely weird error. This should never happen.
log.Printf(
"gotk4: critical: finishRemovingToggleRef called on unknown GObject %s",
objInfo(gobject))
return
}

if strong {
// Panic here, else we're memory leaking.
log.Panicf(
"gotk4: critical: finishRemovingToggleRef cannot be called on strong GObject %s (object unexpectedly resurrected?)",
objInfo(gobject))
}

// If the closures are weak-referenced, then the object reference hasn't
// been toggled yet. Since the object is going away and we're still
// weakly referenced, we can wipe the closures away.
//
// Finally clear the object data off the registry.
delete(shared.weak, gobject)
}

0 comments on commit a1bfe15

Please sign in to comment.