search

samdze / playdate-nim

Nim bindings with extra features for the Playdate SDK
66 stars 3 forks source link

sigabrt: double free when calling file.close() #72

Open ninovanhooff opened 6 months ago

ninovanhooff commented 6 months ago

It seems the nim-bindings take care of closing files when they go out of scope. This is kind of nice, but it was unexpected for me.

This creates opportunity for a dev to trigger a double free: when the game-dev uses file.close() the file is free-ed, and subsequently by the nim bindings =destroy again. This could lead to a SIGABRT: double free with a stacktrace that leads to:

/home/nino/.nimble/pkgs/playdate-#main/playdate/file.nim(35) =destroy

Note that this has only been observed when building for Linux, with the fPIC compiler argument. (I added that argument because the compiler itself suggested to do so). When building for mac, win or device, this behavior is not triggerd

I'd say that the bindings should not expose file.close() if it is not intended to be used by game devs. Indeed the example project doesn't use it either.

As it is, I think it is easy to trip up here. Even in menory managed languages like Java, devs are expected to call file.close()

samdze commented 1 month ago

So a few thoughts on this: maybe better not to avoid providing a close function and leave the file closing to the variable going out of scope since a variable could be stored somewhere and never leave its scope.

The simplest viable solution would be to just check whether the file is not already closed when the variable is going out of scope, and close it if not.