C objects are not destroyed when the GC frees the Go object

[emersion]

This results in memory leaks.

The fix is to use runtime.SetFinalizer (see e.g. https://github.com/emersion/go-tsm/blob/master/vte.go#L47).

[alexbakker]

Yeah, almost all memory management in go-wlroots has to be done manually right now.

Fixing this is a bit more complicated than it seems. Right now, all Go structs that wrap C pointers are always passed by value. I did it that way to prevent having multiple different Go pointers for the same C pointer.

To use runtime.SetFinalizer those Go structs should be passed as pointers instead. We'd need to keep track of all C pointer and Go pointer pairs to be able to map C pointers passed through callbacks to the right Go pointer. Some more explanation here:

go-wlroots/wlroots/listener.go

Lines 3 to 18 in b00418e

	// This whole mess has to exist for a number of reasons:
	//
	// 1. We need to allocate all instances of wl_listener on the heap as storing Go
	// pointers in C after a cgo call returns is not allowed.
	//
	// 2. The wlroots library implicitly destroys objects when wl_display is
	// destroyed. So, we need to keep track of all objects (and their listeners)
	// manually and listen for the destroy signal to be able to free everything.
	//
	// 3 (TODO). As we're keeping track of all objects anyway, we might as well
	// store a Go pointer to the wrapper struct along with them in order to be able
	// to pass the same Go pointer through callbacks every time. This will also
	// allow calling runtime.SetFinalizer on some of them to clean them up early
	// when the GC notices it has gone out of scope.
	//
	// Send help.

I'm not sure if there's a better way to do this. Every solution I can think of is ugly and complicated.

[diamondburned]

Am I guessing this correctly that, in this solution, the Go object pointer would
be globally stored until a destroy signal is sent over, which will then pop the
object off and wait for the GC to finalize destroying it?

Also, as a side note, once it's decided that Go structs that wrap C values
should be passed by pointer, it might be worth it to do something like

type Backend struct {
	_ [0]sync.Mutex
	p *C.struct_wlr_backend
}

which will allow the linter to warn the user when Backend is accidentally
dereferenced elsewhere (which will remove the finalizer).