go-wlroots icon indicating copy to clipboard operation
go-wlroots copied to clipboard

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

Open emersion opened this issue 6 years ago • 2 comments

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).

emersion avatar Oct 12 '18 14:10 emersion

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: https://github.com/swaywm/go-wlroots/blob/b00418ee21647ef8a398d9badc7111c1571e4946/wlroots/listener.go#L3-L18

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

alexbakker avatar Oct 13 '18 10:10 alexbakker

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).

diamondburned avatar Dec 02 '21 00:12 diamondburned