sdl2-compat not compatable with SDL2 code. Causes false positives on memory leaks. #255
Comments
|
I converted the above to SDL3 and checked ldd to see if it was linked to sdl3 shared object and it was and not linked with any sdl2 so. The sdl3 version create the same false memory leaks. So it's not just sdl2-compat it's also sdl3 that is having the issue. I understand sdl2-compat is using sdl3, which is why i checked it. |
|
Issue with SyncTERM Input After Arch Update to sdl2-compat Hi, I recently upgraded my Arch system, which replaced sdl2 with sdl2-compat, and I've encountered an issue with SyncTERM, an old-school BBS client. Prior to the update, everything worked as expected, but after switching to sdl2-compat, SyncTERM no longer sends key presses to the connected BBS. I've reached out to the SyncTERM developer, and they're investigating whether something in sdl2-compat may be causing the issue. In the meantime, I wanted to check if this is something you're actively maintaining and, if so, what information you’d need from me to help debug the problem. Please let me know how I can assist in narrowing this down! Thanks! |
I moved this to a new issue, for tracking. |
|
I pushed a fix for one leak that's in sdl2-compat itself, but almost all of these are under SDL_DBus_Init() in SDL3....rather, they are in libdbus, called into from SDL_DBus_Init(). But SDL2 does not trigger all these, so SDL3 is legitimately leaking something somewhere. I also get leaks in Nvidia's drivers that SDL2 doesn't trigger, so let's make sure we didn't, like, fail to delete the GL context in SDL_DestroyRenderer, too. This is also an SDL3 thing, not sdl2-compat. |
|
I opened an issue on Archlinux sdl2-compat asking for sdl2 to brought back at least as an option. https://gitlab.archlinux.org/archlinux/packaging/packages/sdl2-compat/-/issues/1 |
|
Arch can do what it wants, but it's a small, one-time memory leak that we'll probably fix shortly. |
|
I was simply giving info since most of the current bug reports are from arch users who got rug pulled. There working package got replaced with something that doesn't. And there is an influx of people looking for information and help. The issue i raised was bringing back the working package and having this as an alternative since it's not ready to use yet. |
icculus
changed the title
|
More notes on memory leaks: We call XrmInitialize() in the X11 backend, which allocates memory it never frees behind our backs. https://github.com/ghaerr/nxlib/blob/68f61d915b28f00ceaa97301bd5a2c0fb7258f12/Xrm.c#L323-L327 This eventually ends up in a variable they call "neverFreeTable". :/ https://github.com/ghaerr/nxlib/blob/e190e2080ad4808056d2ba208d008f1a25bd4d28/Quarks.c#L108-L129 We probably can't do anything about that except move to Wayland. It's possible that the reason we're using this (DPI detection) isn't necessary or reliable, and we can just scrap it if so: (But again, this is a small leak that happens once at startup, not a massive ongoing bloating of address space.) We also call XSetLocaleModifiers("") and this leaks in a similar way. There doesn't appear to be a way to reset it, and you have to call this function or things will go wrong (according to the manpage). These are Xlib allocations; maybe moving to xcb would allow us to avoid them, but I don't know the details and doing that would be a massive destabilization of our X11 support. The D-Bus leaks are easy: if I export SDL_SHUTDOWN_DBUS_ON_QUIT=1, they go away. We close and deref the D-Bus objects we own, but we don't actually call dbus_shutdown() without that hint, which defaults to false. We unload the D-Bus library no matter what though. I sort of think this hint should default to true; apps that are also using D-Bus outside of SDL should set it explicitly, since they're going to be a minority (and someone should probably file a bug with D-Bus about this interface, too.) |
|
Anyhow, tl;dr: (stop using X11 and then) export SDL_VIDEO_DRIVER=wayland and SDL_SHUTDOWN_DBUS_ON_QUIT=1 and AddressSanitizer will (probably) find zero leaks. |
|
Yeah stop using x11 doesn't sound like a real answer. Unless your going to tell people that sdl3 no longer supports X11. But it sounds like you have some memory leaks that need to be addressed. There is a function that shuts down sdl called SDL_Quit this should be freeing any memory that SDL has allocated. If it's not then you have a bug there. Telling people to not use X11 because you have memory likes is like a doctor telling the patient not to press there because it will hurt. Instead of addressing the issue. There is a lot of projects that are using sdl2 and also using x11. So if sdl3 is broken or doesn't support actual sdl2 code or x11 then sdl2-compat should not really exist since it's not actually targeting the actual sdl2 use case. By the way does xmonad support wayland now? Since Xmonad is my primary desktop. Also does ffmpeg support wayland now? Think it doesn't. |
|
Sorry, I was being glib. The "stop using x11" comment was obviously not intended to be a real solution. But I can't free memory I didn't allocate and don't even have a pointer to. A decades-old library is doing it and offers no way to free it, intentionally, other than terminating the process. The D-Bus thing is tricky, too, because the way you fix the leak is to call a global shutdown function that will break anything else in the app using D-Bus outside of SDL. Again, this is not our code. I'm of the opinion we should take the crash risk here, because it's an uncommon situation, but that's to be decided still. |
|
From the D-Bus documentation:
This is pretty clear that we shouldn't do this by default and that SDL_SHUTDOWN_DBUS_ON_QUIT should only be set by people trying to do memory leak detection. |
|
Sorry i read that dbus as you must call it after releasing all memory. So it should be called by SDL_Quit() So lets check out your suggestions for the best case. My Yellow-Snow game which doesn't produce any memory leaks in Wayland/X11 at all for sdl2. in Wayland with the dbus switch in sdl2-compat it's a different story.SDL_VIDEO_DRIVER=wayland SDL_SHUTDOWN_DBUS_ON_QUIT=1 make rebuild run Conways Game of Life written in both C and Cpp. sdl2 no issues. in sdl2-compatSDL_VIDEO_DRIVER=wayland SDL_SHUTDOWN_DBUS_ON_QUIT=1 make rebuild run Minesweeper again written in both C and C++SDL_VIDEO_DRIVER=wayland SDL_SHUTDOWN_DBUS_ON_QUIT=1 make rebuild run How about an SDL3 application? Beginners Guide to SDL3 in C and also the Cpp version does the same.SDL_VIDEO_DRIVER=wayland SDL_SHUTDOWN_DBUS_ON_QUIT=1 make clean debug run |
|
@ProgrammingRainbow, thank you for setting up a bunch of test cases for us to investigate. We will come back to this, but are prioritizing game breaking bugs over memory leaks at the moment. |
Ugh, okay. :/ |
|
For the XrmInititalize leak, I'm thinking we should try other approaches first (GDK's Xsettings key, if not the GDK_SCALE environment variable), and try Xft.dpi as a last resort, so we don't leak the memory if there was a better option. But I don't really know the ramifications of that, or which of these things are more or less reliable. |
|
Note that Wayland won't check-out perfectly clean either, particularly if the desktop requires libdecor, which loads cairo/GTK for drawing window decorations, which have some leaks of their own. |
Based on the discussion in libsdl-org/SDL#11142, we are checking scale options in the correct order, and Xft.dpi should be preferred. |
|
In general we can't control leaks in other libraries and we shouldn't be doing gyrations to avoid them. Anyone who finds those leaks while investigating them in SDL can report them upstream. |
|
Running --- a/src/core/linux/SDL_dbus.c
+++ b/src/core/linux/SDL_dbus.c
@@ -98,7 +98,7 @@ static bool LoadDBUSSyms(void)
static void UnloadDBUSLibrary(void)
{
if (dbus_handle) {
- SDL_UnloadObject(dbus_handle);
+// SDL_UnloadObject(dbus_handle);
dbus_handle = NULL;
}
}Perhaps AddressSanitizer has special rules to ignore dbus leaks? |
|
All those I posted above where all in wayland with the dbus switch. |
|
I think @madebr is on the right track. It's not that ASan treats D-Bus differently; it treats any shared libraries that have not unloaded as exempt from leak checking. This is sensible because any loaded shared libraries could have legitimate memory allocations owned internally to the library that would be falsely detected as leaks in the application upon exit. Because we're calling See also the upstream report on this behavior which also suggests leaving it loaded: https://gitlab.freedesktop.org/dbus/dbus/-/issues/495 |
|
@ProgrammingRainbow, the memory leaks are much improved in the latest main code for sdl2-compat and SDL3. I tested on Ubuntu 24.04 using X11 and XWayland, and these were my results: Note that I did not set any environment variables. As @Kontrabant noted, libdecor has known memory leaks and is used for window decorations when the Wayland video driver is enabled. What do you see? |
|
Just for grins I ran both SDL2 and sdl2-compat using SDL2: sdl2-compat: However, when using X11 and XWayland I'm not seeing any leaks here. It seems like SDL2 and sdl2-compat are now about the same in terms of memory leaks. |
|
Just for fun I wanted to see how many of the Wayland leaks were from SDL itself. I created a file This will ignore any leaks from the third party libdecor and fontconfig libraries, which are known to leak. Then I ran yellow-snow using this file: Looking at the leaks they seem to be inside the OpenGL driver on my system, and if so, they should go away if we stop unloading the OpenGL driver so asan can track the allocations. So I applied this patch to SDL to do that: diff --git a/src/video/SDL_egl.c b/src/video/SDL_egl.c
index 86502549b..92817f937 100644
--- a/src/video/SDL_egl.c
+++ b/src/video/SDL_egl.c
@@ -284,11 +284,11 @@ void SDL_EGL_UnloadLibrary(SDL_VideoDevice *_this)
}
if (_this->egl_data->egl_dll_handle) {
- SDL_UnloadObject(_this->egl_data->egl_dll_handle);
+ //SDL_UnloadObject(_this->egl_data->egl_dll_handle);
_this->egl_data->egl_dll_handle = NULL;
}
if (_this->egl_data->opengl_dll_handle) {
- SDL_UnloadObject(_this->egl_data->opengl_dll_handle);
+ //SDL_UnloadObject(_this->egl_data->opengl_dll_handle);
_this->egl_data->opengl_dll_handle = NULL;
}
Now the result looks much nicer: So SDL itself seems to be in a pretty good place, even when using the Wayland driver. Cheers! |
|
It looks like Sam made good progress here, but how do we deal with the leaks out of our control? I'd like to find a solution, both so they don't bother ProgrammingRainbow in daily use, but also so we don't get bug reports caused by other libraries as time goes on. Do we want to generalize the dbus_shutdown hint to something like SDL_HINT_ADDRESS_SANITIZER (or detect AddressSanitizer at runtime...?) and not dlclose misbehaving libraries in those cases? It seems wild to avoid leak reports by leaking library handles, and I also want to know when we've caused a legitimate leak in memory allocated on our behalf by the library that we failed to free, so a blanket disabling like this is undesirable. Maybe we add a FAQ that says "use this suppression file" and supply something that picks out the things like the neverFreeTable in Xlib, libdecor's leaks, etc., and not the whole library. But automating this somehow would be nicer...I'll read up on AddressSanitizer best practices when I get a moment. |
System is Archlinux, KDE, X11, AMD ai 9 hx 370 using the 6.13 testing kernel.
Archlinux recently replaced there SDL2 package with SDL2-compat and added SDL3. This seems to have broken all of my SDL2 projects. I compile strict debugging and sanitizer to help to diminish the chance of coding mistakes. Again this seems to have broken all of my projects. So this sdl2-compat is not currently compatible with sdl2 at least not fully. But it seems archlinux no longer has the real sdl2 package.
src/main.h
src/main.c
src/init_sdl.h
src/init_sdl.c
src/game.h
src/game.c
Makefile
The text was updated successfully, but these errors were encountered: