#1355 closed defect (fixed)
[PATCH] Hardware cursors should be disabled before SDL shutdown
Reported by: | historic_bruno | Owned by: | Deiz |
---|---|---|---|
Priority: | Should Have | Milestone: | Alpha 11 |
Component: | Core engine | Keywords: | patch |
Cc: | Zsolt Dollenstein | Patch: |
Description
Since #748, I get this error on Linux when shutting down the game:
TIMER| shutdown SDL: 3.27935 ms TIMER| shutdown UserReporter: 308.556 us TIMER| shutdown ScriptingHost: 1.77063 ms TIMER| shutdown ConfigDB: 0.207 us SDL_GetWMInfo is not implemented on this platform TIMER| resource modules: 2.98616 ms
We shouldn't be using SDL_* functions after it's shut down, instead we should free and disable hardware cursors before SDL.
Here's the call stack:
#0 get_wminfo (wminfo=...) at ../../../source/lib/sysdep/os/unix/x/x.cpp:422 ret = 0 #1 0x0000000000c3b658 in sys_cursor_free (cursor=0x23e07e0) at ../../../source/lib/sysdep/os/unix/x/x.cpp:504 impl = 0x23e07e0 wminfo = {version = {major = 1 '\001', minor = 2 '\002', patch = 14 '\016'}, subsystem = SDL_SYSWM_X11, info = {x11 = { display = 0x8bf2b8, window = 20, lock_func = 0x23cefb0, unlock_func = 0xd34ad4, fswindow = 12841884, wmwindow = 140737184310608, gfxdisplay = 0x1112990}}} #2 0x0000000000be5b7d in Cursor_dtor (c=0x7fffede0a850) at ../../../source/lib/res/graphics/cursor.cpp:187 __func__ = "Cursor_dtor" #3 0x0000000000bddba0 in h_free_hd (hd=0x7fffede0a810) at ../../../source/lib/res/h_mgr.cpp:561 vtbl = 0x110b240 #4 0x0000000000bde5b6 in Shutdown () at ../../../source/lib/res/h_mgr.cpp:784 hd = 0x7fffede0a810 s = {<No data fields>} #5 0x0000000000bb88a4 in ModuleShutdown (initState=0x11129e8, shutdown=0xbde53c <Shutdown()>) at ../../../source/lib/module_init.cpp:71 latchedInitState = 17947872 __func__ = "ModuleShutdown" #6 0x0000000000bde696 in h_mgr_shutdown () at ../../../source/lib/res/h_mgr.cpp:798 No locals. #7 0x000000000090ee1a in Shutdown () at ../../../source/ps/GameSetup/GameSetup.cpp:699 LINE_692_ = {m_t0 = 15.855938576, m_description = 0xcc3fa8 L"resource modules"} #8 0x00000000006fe6ae in RunGameOrAtlas (argc=1, argv=0x7fffffffe238) at ../../../source/main.cpp:527 args = { m_Args = {<std::_Vector_base<std::pair<CStr8, CStr8>, std::allocator<std::pair<CStr8, CStr8> > >> = { _M_impl = {<std::allocator<std::pair<CStr8, CStr8> >> = {<__gnu_cxx::new_allocator<std::pair<CStr8, CStr8> >> = {<No data fields>}, <No data fields>}, _M_start = 0x0, _M_finish = 0x0, _M_end_of_storage = 0x0}}, <No data fields>}, m_Arg0 = { path = {static npos = <optimized out>, _M_dataplus = {<std::allocator<wchar_t>> = {<__gnu_cxx::new_allocator<wchar_t>> = {<No data fields>}, <No data fields>}, _M_p = 0x111b7b8 L"/home/ben/0ad/binaries/system/pyrogenesis_dbg"}}, separator = 47 L'/'}} ran_atlas = false res = 1.0000000000000001e-09 #9 0x00000000006fe97c in main (argc=1, argv=0x7fffffffe238) at ../../../source/main.cpp:569 No locals.
Attachments (1)
Change History (9)
comment:1 by , 12 years ago
Owner: | set to |
---|
follow-up: 3 comment:2 by , 12 years ago
comment:3 by , 12 years ago
Replying to zsol:
I'm not sure what the options are here as I'm not too familiar with this custom resource manager. Freeing the cursors requires the window id, which sys_cursor_* functions get from SDL.
If it's possible, keeping SDL alive while the resources are freed would be the best approach. What do others think?
Hmm shutdown order is important to get right, I'm not sure if SDL depends on any h_mgr handles existing for it's whole lifetime or not. Either way, it seems like there should be a way to only destroy the cursor at the correct time, maybe we need to pull the cursor out of h_mgr (or as discussed elsewhere, extend/replace h_mgr if it doesn't do what we want).
comment:4 by , 12 years ago
Milestone: | Alpha 10 → Alpha 11 |
---|
Doesn't seem to cause major problems, so pushing it to Alpha 11.
comment:5 by , 12 years ago
Keywords: | patch review added |
---|---|
Owner: | changed from | to
Status: | new → assigned |
Summary: | Hardware cursors should be disabled before SDL shutdown → [PATCH] Hardware cursors should be disabled before SDL shutdown |
Pretty straight-forward fix. From what I gather, only lib/res/graphics/cursor.cpp knows the value of H_Cursor, so cursor freeing must start there. From then on, it's basically the for loop from h_mgr's Shutdown() with additional type checking.
Seems to work well for me, but this is a subsystem I'm unfamiliar with, so I'm posting the patch for review.
by , 12 years ago
Attachment: | free-cursors.patch added |
---|
comment:6 by , 12 years ago
I would pull cursor_shutdown
out of ShutdownSDL
and make it a separate call in the shutdown process. Otherwise it's not immediately clear that shutting down SDL would free cursors (since that order would only be strictly necessary on *nix with Xcursor).
It seems to work correctly on Windows with both hardware and GL cursors.
I'm not sure what the options are here as I'm not too familiar with this custom resource manager. Freeing the cursors requires the window id, which sys_cursor_* functions get from SDL.
If it's possible, keeping SDL alive while the resources are freed would be the best approach. What do others think?