Fix self-referencing closure memory leaks

gir/cmd/gir-generate/gendata/gendata.go

@@ -404,11 +404,10 @@ func GLibVariantIter(nsgen *girgen.NamespaceGenerator) error {
 			}
 
 			_variant := (*Variant)(gextras.NewStructNative(unsafe.Pointer(_cret)))
-			runtime.SetFinalizer(
+			runtime.AddCleanup(
 				gextras.StructIntern(unsafe.Pointer(_variant)),
-				func(intern *struct{ C unsafe.Pointer }) {
-					C.g_variant_unref((*C.GVariant)(intern.C))
-				},
+				func(ptr unsafe.Pointer) { C.g_variant_unref((*C.GVariant)(ptr)) },
+				unsafe.Pointer(_cret),
 			)
 			return _variant, nil
 		}
@@ -428,11 +427,10 @@ func GLibVariantIter(nsgen *girgen.NamespaceGenerator) error {
 				}
 
 				variant := (*Variant)(gextras.NewStructNative(unsafe.Pointer(item)))
-				runtime.SetFinalizer(
+				runtime.AddCleanup(
 					gextras.StructIntern(unsafe.Pointer(variant)),
-					func(intern *struct{ C unsafe.Pointer }) {
-						C.g_variant_unref((*C.GVariant)(intern.C))
-					},
+					func(ptr unsafe.Pointer) { C.g_variant_unref((*C.GVariant)(ptr)) },
+					unsafe.Pointer(item),
 				)
 
 				return variant
@@ -563,11 +561,10 @@ func GioArrayUseBytes(nsgen *girgen.NamespaceGenerator) error {
 			)
 
 			_bytes := (*Bytes)(gextras.NewStructNative(unsafe.Pointer(v)))
-			runtime.SetFinalizer(
+			runtime.AddCleanup(
 				gextras.StructIntern(unsafe.Pointer(_bytes)),
-				func(intern *struct{ C unsafe.Pointer }) {
-					C.g_bytes_unref((*C.GBytes)(intern.C))
-				},
+				func(ptr unsafe.Pointer) { C.g_bytes_unref((*C.GBytes)(ptr)) },
+				unsafe.Pointer(v),
 			)
 
 			return _bytes

gir/girgen/generators/union.go

@@ -187,11 +187,12 @@ func (ug *UnionGenerator) Use(union *gir.Union) bool {
 		p.Linef("original := (*C.%s)(%s.underlying%s())", union.CType, ug.Recv(), ug.GoName)
 		p.Linef("copied := C.%s(original)", copyMethod.CIdentifier)
 		p.Linef("dst := (*%s)(gextras.NewStructNative(unsafe.Pointer(copied)))", ug.GoName)
-		p.Linef("runtime.SetFinalizer(")
+		p.Linef("runtime.AddCleanup(")
 		p.Linef("  gextras.StructIntern(unsafe.Pointer(dst)),")
-		p.Linef("  func(intern *struct{ C unsafe.Pointer }) {")
-		p.Linef("    %s", types.RecordPrintFree(ug.gen, &typRecord, "intern.C"))
+		p.Linef("  func(ptr unsafe.Pointer) {")
+		p.Linef("    %s", types.RecordPrintFree(ug.gen, &typRecord, "ptr"))
 		p.Linef("  },")
+		p.Linef("  unsafe.Pointer(copied),")
 		p.Linef(")")
 		p.Linef("return dst")
 
@@ -264,12 +265,12 @@ func (ug *UnionGenerator) Use(union *gir.Union) bool {
 			ug.hdr.Import("unsafe")
 			ug.hdr.ImportCore("gextras")
 
-			p.Linef("runtime.SetFinalizer(")
-			// dst is ASSUMED TO BE A POINTER.
+			p.Linef("runtime.AddCleanup(")
 			p.Linef("  gextras.StructIntern(unsafe.Pointer(dst)),")
-			p.Linef("  func(intern *struct{ C unsafe.Pointer }) {")
-			p.Linef("    %s", types.RecordPrintFree(ug.gen, &typRecord, "intern.C"))
+			p.Linef("  func(ptr unsafe.Pointer) {")
+			p.Linef("    %s", types.RecordPrintFree(ug.gen, &typRecord, "ptr"))
 			p.Linef("  },")
+			p.Linef("  unsafe.Pointer(cpy),")
 			p.Linef(")")
 		}
 

gir/girgen/types/typeconv/c-go.go

@@ -222,11 +222,11 @@ func (conv *Converter) cgoArrayConverter(value *ValueConverted) bool {
 			value.p.Linef("var len C.gsize")
 			// If we're fully getting the backing array, then we can just steal
 			// it (since we own it now), which is less copying.
-			value.p.Linef("p := C.g_byte_array_steal(&%s, &len)", value.In.Name)
-			value.p.Linef("%s = unsafe.Slice((*byte)(p), uint(len))", value.Out.Set)
-			value.p.Linef("runtime.SetFinalizer(&%s, func(v *[]byte) {", value.OutName)
-			value.p.Linef("  C.free(unsafe.Pointer(&(*v)[0]))")
-			value.p.Linef("})")
+			value.p.Linef("p := (*byte)(C.g_byte_array_steal(&%s, &len))", value.In.Name)
+			value.p.Linef("%s = unsafe.Slice(p, uint(len))", value.Out.Set)
+			value.p.Linef("runtime.AddCleanup(&%s, func(v *[]byte) {", value.OutName)
+			value.p.Linef("  C.free(unsafe.Pointer(p))")
+			value.p.Linef("}, p)")
 			value.p.Ascend()
 			return true
 		}
@@ -431,18 +431,15 @@ func (conv *Converter) cgoConverter(value *ValueConverted) bool {
 		value.header.NeedsExternGLib()
 		value.header.NeedsGLibObject()
 
-		value.p.LineTmpl(value, `<.Out.Set> =
-			<- .OutPtr 1>coreglib.ValueFromNative(unsafe.Pointer(<.InNamePtr 1>))`)
-
-		// Set this to be freed if we have the ownership now.
+		valueFunc := "ValueFromNative"
 		if value.ShouldFree() {
-			value.header.Import("runtime")
-
-			// https://pkg.go.dev/github.com/diamondburned/gotk4/pkg/core/glib?utm_source=godoc#Value
-			value.p.Linef("runtime.SetFinalizer(%s, func(v *coreglib.Value) {", value.OutName)
-			value.p.Linef("  C.g_value_unset((*C.GValue)(unsafe.Pointer(v.Native())))")
-			value.p.Linef("})")
+			valueFunc = "ValueFromNativeOwned"
 		}
+
+		value.p.Printf("%s =", value.Out.Set)
+		value.p.Printf("  %scoreglib.%s(unsafe.Pointer(%s))", value.OutPtr(1), valueFunc, value.InNamePtr(1))
+		value.p.EmptyLine()
+
 		return true
 
 	case "GObject.Object", "GObject.InitiallyUnowned":
@@ -502,11 +499,11 @@ func (conv *Converter) cgoConverter(value *ValueConverted) bool {
 			value.p.Linef("C.%s(%s)", ref, value.InNamePtr(1))
 		}
 
-		value.p.Linef("runtime.SetFinalizer(%s%s, func(v %s%s) {",
-			value.OutInPtr(1), value.OutName, value.OutPtr(1), value.Out.Type)
-		value.p.Linef("C.%s((%s%s)(unsafe.Pointer(v.Native())))",
-			unref, value.InPtr(1), value.In.Type)
-		value.p.Linef("})")
+		value.p.Linef("runtime.AddCleanup(")
+		value.p.Linef("  %s%s,", value.OutInPtr(1), value.OutName)
+		value.p.Linef("  func(p unsafe.Pointer) { C.%s((%s%s)(p)) },", unref, value.InPtr(1), value.In.Type)
+		value.p.Linef("  unsafe.Pointer(%s),", value.InNamePtr(1))
+		value.p.Linef(")")
 
 		return true
 	}
@@ -642,17 +639,10 @@ func (conv *Converter) cgoConverter(value *ValueConverted) bool {
 		// We can take ownership if the type can be reference-counted anyway.
 		if value.ShouldFree() || unref {
 			value.header.Import("runtime")
-			value.vtmpl(`
-				runtime.SetFinalizer(
-					gextras.StructIntern(unsafe.Pointer(<.OutInPtr 1><.OutName>)),
-					func(intern *struct{ C unsafe.Pointer }) {
-			`)
-			if value.fail {
-				value.Logln(logger.Debug, "SetFinalizer set fail")
-			}
-
-			value.p.Linef("    %s", types.RecordPrintFree(value.conv.fgen, value.Resolved.Extern, "intern.C"))
-			value.p.Linef("  },")
+			value.p.Linef("runtime.AddCleanup(")
+			value.p.Linef("  gextras.StructIntern(unsafe.Pointer(%s%s)),", value.OutInPtr(1), value.OutName)
+			value.p.Linef("  func(ptr unsafe.Pointer) { %s },", types.RecordPrintFree(value.conv.fgen, value.Resolved.Extern, "ptr"))
+			value.p.Linef("  unsafe.Pointer(%s),", value.InNamePtr(1))
 			value.p.Linef(")")
 		}
 

gir/girgen/types/typeconv/go-c.go

@@ -651,18 +651,6 @@ func (conv *Converter) gocConverter(value *ValueConverted) bool {
 		value.vtmpl(
 			"<.Out.Set> = <.OutCast 1>(gextras.StructNative(unsafe.Pointer(<.InNamePtr 1>)))",
 		)
-
-		// If ShouldFree is true, then ideally, we'll be freeing the C copy of
-		// the value once we're done. However, since the C code is taking
-		// ownership, we can't do that, since the Finalizer won't know that and
-		// free the record. Instead, if we cannot free the data once we're done,
-		// then we detach the finalizer so Go can't.
-		if !value.ShouldFree() && types.RecordHasRef(v) == nil {
-			value.header.Import("runtime")
-			value.vtmpl(
-				"runtime.SetFinalizer(gextras.StructIntern(unsafe.Pointer(<.InNamePtr 1>)), nil)",
-			)
-		}
 		return true
 
 	case *gir.Callback:

gir/girgen/types/typeconv/valueconverted.go

@@ -816,7 +816,7 @@ func (value *ValueConverted) OutInNamePtr(want int) string {
 }
 
 // OutInPtr returns the left-hand side for the output name and type SPECIFICALLY
-// for inputting the output name elsewhere, like giving it to SetFinalizer.
+// for inputting the output name elsewhere, like giving it to AddCleanup.
 func (value *ValueConverted) OutInPtr(want int) string {
 	has := types.CountPtr(value.Out.Type)
 	return value._ptr(has, want)

pkg/core/closure/closure.go

@@ -33,3 +33,14 @@ func (r *Registry) Load(gclosure unsafe.Pointer) *FuncStack {
 func (r *Registry) Delete(gclosure unsafe.Pointer) {
 	r.reg.Delete(uintptr(gclosure))
 }
+
+// Len returns the number of closures in the registry.
+// Use this for debugging purposes only.
+func (r *Registry) Len() int {
+	var i int
+	r.reg.Range(func(_, _ interface{}) bool {
+		i++
+		return true
+	})
+	return i
+}

pkg/core/gdebug/objectinfo.go

@@ -0,0 +1,27 @@
+package gdebug
+
+// #cgo pkg-config: gobject-2.0
+// #include <glib-object.h>
+//
+// const gchar *gotk4_object_type_name(gpointer obj) {
+//   return G_OBJECT_TYPE_NAME(obj);
+// };
+import "C"
+import (
+	"fmt"
+	"log/slog"
+	"unsafe"
+)
+
+// ObjectInfo returns a slog.Attr with the GObject's pointer and type.
+func ObjectInfo(obj unsafe.Pointer) slog.Attr {
+	return slog.Group(
+		"gobject",
+		slog.String("ptr", fmt.Sprintf("%p", obj)),
+		slog.String("type", C.GoString(C.gotk4_object_type_name(C.gpointer(obj)))),
+		slog.Int("refs", objRefCount(obj)))
+}
+
+func objRefCount(obj unsafe.Pointer) int {
+	return int(C.g_atomic_int_get((*C.gint)(unsafe.Pointer(&(*C.GObject)(obj).ref_count))))
+}