Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RWFromMem should add finalizer to prevent gc of byte array #614

Open
kazzmir opened this issue Dec 13, 2024 · 1 comment
Open

RWFromMem should add finalizer to prevent gc of byte array #614

kazzmir opened this issue Dec 13, 2024 · 1 comment

Comments

@kazzmir
Copy link

kazzmir commented Dec 13, 2024

Go version: go version go1.23.0 linux/amd64
Go-SDL2 version: v0.4.40
SDL2 version: 2.30
OS: linux
Architecture: amd64

sdl.RWFromMem() takes a []byte array and creates an RWops from it. If the []byte array is garbage collected before the RWops is, then the code/objects using the RWops will most likely crash the program. Adding a finalizer in sdl.RWFromMem() to the returned rwops that references the mem argument would keep mem from being garbage collected too early.

func RWFromMem(mem []byte) (*RWops, error){
  ...
  // this finalizer can be added
  runtime.SetFinalizer(rwops, func (rw *RWops){
    mem = nil // keep reference in the finalizer itself, which will prevent mem from being gc'd. When this finalizer runs, the mem reference will be removed and thus become a candidate for garbage collection
  })
  return rwops, nil
}

I ran into this while trying to load a ttf font. My code looked roughly like this (psuedo-ish without errors)

func loadFont(reader io.Reader, size int) *ttf.Font {
  data := io.ReadAll(reader)
  rwops := sdl.RWFromMem(data)
  return ttf.OpenFontRW(rwops, 1, size)
}

This code would mysteriously cause the font to have garbage data in it at random until I added a finalizer to keep a reference to data

func loadFont(reader io.Reader, size int) *ttf.Font {
  data := io.ReadAll(reader)
  rwops := sdl.RWFromMem(data)
  font := ttf.OpenFontRW(rwops, 1, size)
  runtime.SetFinalizer(font, func(font *ttf.Font){
     data = nil
  })
  return font
}

The TTF docs say that the RWops must have a longer lifetime than the font itself, so the underlying []byte array must also live at least as long as the font.

Since RWFromMem basically depends on the underlying []byte array existing, it seems reasonable to force a reference to the []byte array to be alive while the RWops is alive. I can make a pr if you don't see any issue with this.

@kazzmir
Copy link
Author

kazzmir commented Dec 13, 2024

Actually I made a mistake. It turns out that you cannot add a finalizer directly to an *RWops because the RWops is not allocated by go, but rather by C malloc. Attempting to add a finalizer to the *RWops results in the error

fatal error: runtime.SetFinalizer: pointer not in allocated block

Perhaps the golang sdl library could wrap the underlying *RWops in a golang struct to allocate an object known to the garbage collector?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant