[PATCH] Hardware cursors should be disabled before SDL shutdown

[historic_bruno]

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.

[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?

[historic_bruno]

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).

[historic_bruno]

Doesn't seem to cause major problems, so pushing it to Alpha 11.

[Deiz]

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.

[historic_bruno]

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.

[Deiz]

In 12271:

Free cursors before shutting down SDL. Fixes #1355.

[Deiz]

Committed it with your suggested change.