riksweeney / edgar

A 2D platform game with a persistent world. When Edgar's father fails to return home after venturing out one dark and stormy night, Edgar fears the worst: he has been captured by the evil sorcerer who lives in a fortress beyond the forbidden swamp.
https://www.parallelrealities.co.uk/games/edgar
116 stars 28 forks source link

Issue with atexit() on OpenBSD #50

Closed lumidify closed 3 years ago

lumidify commented 3 years ago

When the program exits on OpenBSD, it always crashes with a bus error due to SDL_DestroyRenderer (in src/init.c:cleanup()) failing. I tracked the error all the way to the call to SDL_GL_DeleteContext in the SDL source code (src/render/opengl/SDL_render_gl.c), but I have no idea what the actual issue is. I just know that it does work without crashing when atexit() is removed and instead cleanup() is called directly.

In the OpenBSD man page for atexit(), it says

atexit() is very difficult to use correctly without creating exit(3)-time races. Unless absolutely necessary, please avoid using it.

That leads me to believe that there is probably some issue with the way cleanup() is called by atexit() in the OpenBSD implementation. I don't know if this only happens on OpenBSD or also on some other systems.

Is there any reason for using atexit(), other than convenience? Would there be any interest in removing it and instead calling a cleanup function directly? Otherwise, I'll just patch it locally so it works properly for me.

riksweeney commented 3 years ago

I could certainly look into it, but isn't this a problem with OpenBSD?

polluks commented 3 years ago

See also https://www.mulle-kybernetik.com/weblog/2019/atexit_is_broken.html

lumidify commented 3 years ago

I'm not entirely sure if it's a problem with OpenBSD or if there is some undefined behavior in there that's just handled differently in OpenBSD - I don't know enough about the official standard to have an opinion on that. If you'd like, I could try to fix this and submit a pull request. As far as I can see, the only changes would be to replace all exit() calls with calls to a function that just calls cleanup() and exit() afterwards. If you want to leave it this way, though, I wouldn't mind just patching it locally.

riksweeney commented 3 years ago

I'll take a look into this.

lumidify commented 3 years ago

I can confirm everything now works fine on OpenBSD. Thanks!

Just for the record: I found a similar problem, in which the issue was that the graphics driver had also registered an exit handler: https://sourceforge.net/p/freeglut/bugs/206/. I don't know what exactly the particular driver used on my machine by OpenBSD does, but the problem was fixed if the atexit() call was moved after the SDL initialization, similar to what was done to fix the bug in freeglut I linked. I guess it wasn't really an issue with the game then and could have been fixed more easily, so sorry for the perhaps unnecessary work!