You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
SdlDrop is hidden from documentation but is still publicly exported. It can be used to call SDL_Quit in safe code, causing undefined behavior when using objects that are expected to still be valid. Example:
{let _ = sdl2::SdlDrop;}// Invokes SDL_Quit
To fix this, I recommend changing the visibility of SdlDrop to pub(crate). If it's intended to be used by outside crates like other SDL extension bindings, then I'd recommend protecting SdlDrop behind the IS_SDL_CONTEXT_ALIVE flag instead of Sdl. If any of these changes are desired, I'll open a pull request.
The text was updated successfully, but these errors were encountered:
I think it's intended to be pub for other modules, (it was probably used when sdl2-ttf, sdl2-image and so on were in separate repositories).
I think we can do better than that though for a fix.
The current solution in the repository uses an atomic boolean and an Rc<_> to decrease a count, when it reaches 0 SDL is destroyed with a SdlDrop. But since rust 1.34.0 we have Atomic integers.
Instead of having an Rc<SdlDrop>, we could have a SdlDrop, which on creation would increase the counter by one, and on Drop decrease it by one. Once the counter reaches 0, call SDL_Quit.
Since it might be used by other libraries, we need to keep SdlDrop public. But we could make it impossible to build without a ref to &Sdl. Like pub struct SdlDrop { _anticonstructor: () }, and like in the actual code it could only be created via sdl.sdldrop().
That way no matter how many SdlDrop structs are created, it will always be safe.
Uh oh!
There was an error while loading. Please reload this page.
SdlDrop
is hidden from documentation but is still publicly exported. It can be used to callSDL_Quit
in safe code, causing undefined behavior when using objects that are expected to still be valid. Example:To fix this, I recommend changing the visibility of
SdlDrop
topub(crate)
. If it's intended to be used by outside crates like other SDL extension bindings, then I'd recommend protectingSdlDrop
behind theIS_SDL_CONTEXT_ALIVE
flag instead ofSdl
. If any of these changes are desired, I'll open a pull request.The text was updated successfully, but these errors were encountered: