Skip to content

Commit 96fbd78

Browse files
committed
Attempt to fix GC cyclic reference leaking
This commit adds a new dummy pointer field into the `intern.Box` structure that points to a non-GC'd uintptr value (in this case the GObject address). Using this new dummy pointer, we can attach a finalizer to this value instead of the actual `intern.Box` value. This is done because `intern.Box` also contains a strong reference to the object's closure callbacks, which in turn hold a strong reference to `intern.Box`, causing a cyclic reference. Because the dummy pointer value points to a uintptr, it never references anything else, so there is no GC cycle. We can then use our old Finalizer trick to try and resurrect the `intern.Box` from the dummy pointer and check if it should be freed in Go's heap as well. With this, memory leaks should not happen anymore.
1 parent 18db6bb commit 96fbd78

File tree

2 files changed

+49
-14
lines changed

2 files changed

+49
-14
lines changed

pkg/core/glib/glib.go

-1
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,6 @@ func ConnectGeneratedClosure(
507507
defer C.free(unsafe.Pointer(csignal))
508508

509509
id := C.g_signal_connect_closure(C.gpointer(v.native()), csignal, gclosure, gbool(after))
510-
C.g_closure_sink(gclosure)
511510

512511
runtime.KeepAlive(obj)
513512
return SignalHandle(id)

pkg/core/intern/intern.go

+49-13
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import (
2626

2727
const maxTypesAllowed = 3
2828

29-
var knownTypes uint32
29+
var knownTypes uint32 = 0
3030

3131
type BoxedType[T any] struct {
3232
ctor func(*Box) *T
@@ -81,7 +81,8 @@ func (t *BoxedType[T]) Delete(box *Box) {
8181

8282
// Box is an opaque type holding extra data.
8383
type Box struct {
84-
data [maxTypesAllowed]unsafe.Pointer
84+
dummy *unsafe.Pointer
85+
data [maxTypesAllowed]unsafe.Pointer
8586
}
8687

8788
// Object returns Box's C GObject pointer.
@@ -114,6 +115,11 @@ func newBox(obj unsafe.Pointer) *Box {
114115
box := &Box{}
115116
box.data[0] = obj
116117

118+
// Cheat Go's GC by adding a finalizer to a dummy pointer that is inside Box
119+
// but is not Box itself.
120+
box.dummy = &obj
121+
runtime.SetFinalizer(box.dummy, finalizeBox)
122+
117123
if objectProfile != nil {
118124
objectProfile.Add(obj, 3)
119125
}
@@ -189,7 +195,6 @@ func Get(gobject unsafe.Pointer, take bool) *Box {
189195
// reverse link is always strong.
190196
//
191197
shared.strong[gobject] = box
192-
runtime.SetFinalizer(box, finalizeBox)
193198

194199
shared.mu.Unlock()
195200

@@ -259,38 +264,60 @@ func Free(box *Box) {
259264
}
260265
}
261266

267+
var finalizing atomic.Bool
268+
262269
// finalizeBox only delays its finalization until GLib notifies us a toggle. It
263270
// does so for as long as an object is stored only in the Go heap. Once the
264271
// object is also shared, the toggle notifier will strongly reference the Box.
265-
func finalizeBox(box *Box) {
266-
obj := box.GObject()
267-
if obj == nil {
268-
return
272+
func finalizeBox(dummy *unsafe.Pointer) {
273+
// obj := box.GObject()
274+
// if obj == nil {
275+
// return
276+
// }
277+
278+
// var objInfoRes string
279+
// if toggleRefs != nil {
280+
// objInfoRes = objInfo(obj)
281+
// toggleRefs.Println(objInfoRes, "finalizeBox: acquiring lock...")
282+
// }
283+
284+
shared.mu.Lock()
285+
286+
if !finalizing.CompareAndSwap(false, true) {
287+
panic("impossible: finalizeBox called while finalizing")
288+
}
289+
defer finalizing.Store(false)
290+
291+
box, _ := gets(*dummy)
292+
if box == nil {
293+
panic("bug: finalizeBox called on already freed object")
269294
}
270295

296+
obj := box.GObject()
297+
271298
var objInfoRes string
272299
if toggleRefs != nil {
273300
objInfoRes = objInfo(obj)
274301
toggleRefs.Println(objInfoRes, "finalizeBox: acquiring lock...")
275302
}
276303

277-
shared.mu.Lock()
278-
279304
if !freeBox(box) {
280305
// Delegate finalizing to the next cycle.
281-
runtime.SetFinalizer(box, finalizeBox)
306+
runtime.SetFinalizer(box.dummy, finalizeBox)
282307
shared.mu.Unlock()
308+
283309
if toggleRefs != nil {
284310
toggleRefs.Println(objInfoRes, "finalizeBox: moving finalize to next GC cycle")
285311
}
286312
} else {
287-
shared.mu.Unlock()
288313
// Unreference the object. This will potentially free the object as
289314
// well. The closures are definitely gone at this point.
290315
C.g_object_remove_toggle_ref(
291316
(*C.GObject)(unsafe.Pointer(obj)),
292317
(*[0]byte)(C.goToggleNotify), nil,
293318
)
319+
shared.mu.Unlock()
320+
294321
if toggleRefs != nil {
295322
toggleRefs.Println(objInfoRes, "finalizeBox: removed toggle ref during GC")
296323
}
@@ -346,15 +373,18 @@ func goToggleNotify(_ C.gpointer, obj *C.GObject, isLastInt C.gboolean) {
346373
gobject := unsafe.Pointer(obj)
347374
isLast := isLastInt != C.FALSE
348375

349-
shared.mu.Lock()
376+
if !finalizing.Load() {
377+
shared.mu.Lock()
378+
defer shared.mu.Unlock()
379+
}
380+
350381
if isLast {
351382
// delete(shared.sharing, gobject)
352383
makeWeak(gobject)
353384
} else {
354385
// shared.sharing[gobject] = struct{}{}
355386
makeStrong(gobject)
356387
}
357-
shared.mu.Unlock()
358388

359389
if toggleRefs != nil {
360390
toggleRefs.Println(objInfo(unsafe.Pointer(obj)), "goToggleNotify: is last =", isLast)
@@ -388,6 +418,9 @@ func makeStrong(gobject unsafe.Pointer) {
388418
// TODO: double mutex check, similar to ShouldFree.
389419

390420
box, strong := gets(gobject)
421+
if toggleRefs != nil {
422+
toggleRefs.Println(objInfo(gobject), "makeStrong: obtained box", box, "strong =", strong)
423+
}
391424
if box == nil {
392425
return
393426
}
@@ -402,6 +435,9 @@ func makeStrong(gobject unsafe.Pointer) {
402435
// weakly referenced.
403436
func makeWeak(gobject unsafe.Pointer) {
404437
box, strong := gets(gobject)
438+
if toggleRefs != nil {
439+
toggleRefs.Println(objInfo(gobject), "makeWeak: obtained box", box, "strong =", strong)
440+
}
405441
if box == nil {
406442
return
407443
}

0 commit comments

Comments
 (0)