Widget memory is not freed when removed

[jgillich]

I've noticed that memory usage increases over time in my application, and it seems to be directly related to the replacement of widgets. A GtkColumnView does not leak memory when re-rendering, but when I remove and recreate widgets myself, it appears that the old ones are not freed. Here's an example program:

package main

import (
	"fmt"
	"os"
	"runtime"

	"github.com/diamondburned/gotk4/pkg/gio/v2"
	"github.com/diamondburned/gotk4/pkg/gtk/v4"
	"github.com/pkg/profile"
)

func main() {
	gtk.Init()

	app := gtk.NewApplication("foo.bar.baz", gio.ApplicationFlagsNone)

	app.ConnectActivate(func() {

		w := gtk.NewApplicationWindow(app)
		w.SetApplication(app)
		w.SetSizeRequest(400, 400)
		defer w.Show()

		s := profile.Start(profile.MemProfile)
		w.ConnectCloseRequest(func() (ok bool) {
			s.Stop()
			return false
		})

		root := gtk.NewBox(gtk.OrientationVertical, 4)
		root.SetMarginTop(4)
		root.SetMarginStart(4)
		root.SetMarginEnd(4)
		w.SetChild(root)

		btn := gtk.NewButton()
		btn.SetLabel("Leak memory")
		root.Append(btn)

		sw := gtk.NewScrolledWindow()
		root.Append(sw)
		items := gtk.NewBox(gtk.OrientationVertical, 4)
		items.SetVExpand(true)
		sw.SetChild(items)

		btn.ConnectClicked(func() {
			for {
				c := items.FirstChild()
				if c != nil {
					items.Remove(c)
				} else {
					break
				}
			}
			for i := 0; i < 1000; i++ {
				items.Append(gtk.NewLabel(fmt.Sprintf("Label %v", i)))
			}
			runtime.GC()
		})
	})

	app.Run(os.Args)
}

With enough clicks, you can reach one gigabyte and more. The widgets aren't referenced after being removed so I'd expect GC to trigger, but it doesn't.

[diamondburned]

Could you try running the program with this?

GOTK4_DEBUG=trace-objects,toggle-refs go run -v .

This will cause gotk4 to give you a log file that you can tail -f with to see the live output. You can then click the button (probably once) and call GC() to see if they're being freed at all.

[diamondburned]

It might also be worth it to run runtime/pprof on the heap to see if the memory bloat is coming from the Go heap or the C heap.

[diamondburned]

Also as a side note, it can be really hard for Go's GC to know when to actually run, because the bulk of the allocations are on the C heap. For this reason, gotkit/gtkutil/aggressivegc exists.

[jgillich]

I can manually trigger GC on every button click and it makes little difference, memory still goes up each time. Tried looking at the log, but I don't see anything useful in there. Edit: is it supposed to show when an object is freed? All I see is these two lines being repeated:

2024/02/02 23:05:35 0x1effb50 (GtkLabel): Get: will introduce new box, current ref = 1
2024/02/02 23:05:35 0x1effb50 (GtkLabel): Get: introduced new box, current ref = 2

Here's a pprof graph with total memory being a few hundred megs:

And here's the same with memory >1GB:

So while most of the memory is C heap, I think it's Go that's keeping the references alive. But this stuff's a little out of my league 😄

Also updated the code above with pprof and GC calls if you want to try for yourself.

[diamondburned]

Edit: is it supposed to show when an object is freed? All I see is these two lines being repeated:

2024/02/02 23:05:35 0x1effb50 (GtkLabel): Get: will introduce new box, current ref = 1
2024/02/02 23:05:35 0x1effb50 (GtkLabel): Get: introduced new box, current ref = 2

Yeah, this isn't good. The debug is supposed to say when objects are being freed. I'm busy this weekends but I will look into this as soon as I can. I think the same bug is appearing on gtkcord4 as well.

[diamondburned]

I figured it out! The ref-counting for floating references was all out of wack.

[diamondburned]

--- a/pkg/core/intern/intern.go
+++ b/pkg/core/intern/intern.go
        // We should already have a strong reference. Sink the object in case. This
        // will force the reference to be truly strong.
        if C.g_object_is_floating(C.gpointer(gobject)) != C.FALSE {
+               // First, we need to ref_sink the object to convert the floating
+               // reference to a strong reference.
                C.g_object_ref_sink(C.gpointer(gobject))
+               // Then, we need to unref it to balance the ref_sink.
+               C.g_object_unref(C.gpointer(gobject))
+
+               if toggleRefs != nil {
+                       toggleRefs.Println(objInfo(gobject),
+                               "Get: ref_sink'd the object, current ref =", objRefCount(gobject))
+               }
        }

[diamondburned]

As a side note, using toggle references for all objects likely kills performance a lot. It's much more ideal if they're only used if we're working with closures attached to objects.

[jgillich]

Awesome, that fixes it! Thank you

[diamondburned]

Reopening due to a regression.

[abenz1267]

would this cause common.items.Splice(0, int(common.items.NItems()), handler.entries...) to leak as well? Or is that a different issue eventually?

[diamondburned]

The splice call likely isn't the cause, it's probably handler.entries. Unfortunately, I don't currently have time to experiment with this :(

[abenz1267]

Hm... because if i remove the splice call, memory is fine.

Edit: I've switched handler.entries to a newly created inline struct slice, the more slice items, the more memory leakage. I do suspect indeed splice to be the issue... or widget memory as this issue suggests. Maybe I'm missing something.

[diamondburned]

Ah, it looks like gioutil.ListModel leaks after all, but thankfully not related to this issue! This issue is a much more tedious bug that involves GC cyclic references, while ListModel just leaks because it's missing some gbox calls to free up items.

Could you open a separate issue for this?

[abenz1267]

would somekinda bug bounty be an option? is anyone else able to fix this?

i'm afraid money doesn't change the fact @diamondburned has no time to fix this.... someone else?

[diamondburned]

Actually, once golang/go#67535 lands in Go 1.24, I'll definitely be fixing this problem, now that it's... actually fixable.

[diamondburned]

Words alone cannot express how excited I am for this feature to land so I can finally fix this years-old bug in gotk4 that I spent years thinking about @@