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

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

Open
ninovanhooff opened this issue Apr 28, 2024 · 1 comment
Open

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

ninovanhooff opened this issue Apr 28, 2024 · 1 comment

Comments

@ninovanhooff
Copy link
Collaborator

ninovanhooff commented Apr 28, 2024

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
Copy link
Owner

samdze commented Oct 12, 2024

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.

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

2 participants