wesnoth
GUI2 event handling causes crash if modeless dialog is closed in key press callback
Did some debugging. Looks like it has to do with more events being executed by the dispatcher on the destroyed dialog object after it closes. Not sure why. Setting handled and halt in the two key press callbacks fixes some of the cases of the crash, but not all.
More investigating shows this has to do with the modeless_dialog's window object being destroyed in the key callback. Something is trying to execute a SDL_KEYDOWN event on (presumably) the dialog's window object after it's been destroyed... It looks like the result of keyboard_dispatcher() points to an already-deleted widget, but I can't figure out how that would be, since at the same time debugging shows no dispatchers registered.
Ok, turns out I was debugging incorrectly (thanks to celmin for pointing it out). Also turns out there are 4 events fired for a single key press in the text field:
- SDL_KEYDOWN to the window
- SDL_KEYDOWN to the text box
- SDL_TEXTINPUT to the window
- SDL_TEXTINPUT to the text box
Since the window is destroyed after 2, 3 and on crash. This happens because SDL_TEXTINPUT events also fire SDL_KEYDOWN callbacks... FML.
That's on pressing Enter, for the record. Pressing Esc seems to generate all SDL_KEYDOWN events and the crash happens on the third one.
So basically we need to ensure that the third and fourth are cancelled. I think you mentioned that you already tried setting halt and handled to false and it didn't help?
Did more research last night. It seems that Esc and Enter might actually trigger all key down events, instead of 2 key down, 2 text input. But I still can't pinpoint where exactly the bug happens...
During debugging, I noticed that indeed, handled and halt were set to true and exited the event loop. The problem seems to be that the event loop gui2::event::implementation::fire_event is entered again, at which point it crashes. That is backed up by handled and halt being false at the time of the crash. Given that events seem to be executed on the window, then the text box, it seems logically, the crash would be when trying to execute event 3. But I also added logging in the GUI2 event handler pipeline (gui2::event::sdl_event_handler::key_down) and I didn't see any of that before the crash. It does appear to be handling an SDL_KEYDOWN event (the SDL_Event), but why did my logging not get printed to console?
Furthermore, the dispatcher dtor (widget inherits from dispatcher) unregisters this from the list of available dispatchers in sdl_event_handler. That successfully happens, so even if event 3 were executing, it should never go to the window! Jyrki suggested that perhaps we're keeping a dispatcher pointer somewhere, but I checked, and forcibly clearing the two pointers I could find (sdl_event_handler::mouse_focus and sdl_event_handler::keyboard_focus_) from the dispatcher dtor had no effect.
It seems I might have been mistaken before that this had to do with text input events, but I'm even more confused now and totally at loss. :(
Even more research shows that indeed the crash happens on event 3. It seems events 1 and 3 take a simple call path via the dispatcher, while event 2 goes through the distributor... I don't understand the distributor at all.
Here's the stacktrace for the crash when executing event 3. Perhaps something about the fact that the distributor is involved with the execution of event 2 explains why we execute event 3 at all... not sure what, though.
> wesnoth.exe!std::_Func_class<void,gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>::_Getimpl() Line 1441 C++
wesnoth.exe!std::_Func_class<void,gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>::_Empty() Line 1283 C++
wesnoth.exe!std::_Func_class<void,gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>::operator()(gui2::widget & <_Args_0>, gui2::event::ui_event <_Args_1>, bool & <_Args_2>, bool & <_Args_3>, int <_Args_4>, SDL_Keymod <_Args_5>, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & <_Args_6>) C++
wesnoth.exe!gui2::event::implementation::fire_event<std::function<void __cdecl(gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &)>,int const &,enum SDL_Keymod const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>(const gui2::event::ui_event event, std::vector<std::pair<gui2::widget *,enum gui2::event::ui_event>,std::allocator<std::pair<gui2::widget *,enum gui2::event::ui_event> > > & event_chain, gui2::widget * dispatcher, gui2::widget * w, const int & <params_0>, const SDL_Keymod & <params_1>, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & <params_2>) Line 332 C++
wesnoth.exe!gui2::event::fire_event<std::function<void __cdecl(gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &)>,int const &,enum SDL_Keymod const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>(const gui2::event::ui_event event, gui2::event::dispatcher * d, gui2::widget * w, const int & <params_0>, const SDL_Keymod & <params_1>, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & <params_2>) Line 406 C++
wesnoth.exe!gui2::event::dispatcher::fire(const gui2::event::ui_event event, gui2::widget & target, const int key, const SDL_Keymod modifier, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & unicode) Line 100 C++
wesnoth.exe!gui2::event::sdl_event_handler::key_down(const int key, const SDL_Keymod modifier, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & unicode) Line 730 C++
wesnoth.exe!gui2::event::sdl_event_handler::key_down(const SDL_Event & event) Line 682 C++
wesnoth.exe!gui2::event::sdl_event_handler::handle_event(const SDL_Event & event) Line 373 C++
wesnoth.exe!events::run_event_loop() Line 617 C++
wesnoth.exe!controller_base::play_slice(bool is_delay_enabled) Line 264 C++
wesnoth.exe!play_controller::play_slice_catch() Line 1058 C++
wesnoth.exe!playsingle_controller::play_human_turn() Line 466 C++
wesnoth.exe!playsingle_controller::play_side_impl() Line 387 C++
wesnoth.exe!play_controller::play_side() Line 1147 C++
wesnoth.exe!play_controller::play_turn() Line 1189 C++
wesnoth.exe!playsingle_controller::play_scenario_main_loop() Line 174 C++
wesnoth.exe!playsingle_controller::play_scenario(const config & level) Line 266 C++
wesnoth.exe!campaign_controller::playsingle_scenario(end_level_data & end_level) Line 205 C++
wesnoth.exe!campaign_controller::play_game() Line 289 C++
wesnoth.exe!game_launcher::launch_game(game_launcher::RELOAD_GAME_DATA reload) Line 960 C++
wesnoth.exe!do_gameloop(const std::vector<std::basic_string<char,std::char_traits<char>,std::allocator<char> >,std::allocator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > > > & args) Line 887 C++
wesnoth.exe!SDL_main(int argc, char * * argv) Line 1063 C++
wesnoth.exe!main_getcmdline(...) Line 161 C
wesnoth.exe!WinMain(HINSTANCE__ * hInst, HINSTANCE__ * hPrev, char * szCmdLine, int sw) Line 203 C
[External Code]
[Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]
@Vultraz I think you're looking too high up in the call stack.
If the problem is that the only event dispatcher is destroyed, then by far the most suspicious function call is these two frames:
wesnoth.exe!gui2::event::dispatcher::fire(const gui2::event::ui_event event, gui2::widget & target, const int key, const SDL_Keymod modifier, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & unicode) Line 100 C++
wesnoth.exe!gui2::event::sdl_event_handler::key_down(const int key, const SDL_Keymod modifier, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & unicode) Line 730 C++
dispatcher::fire() is called even though there are no valid dispatchers left. This should never happen.
To investigate this, you could set up a breakpoint that fires when the window has destroyed itself. At that point it has already unregistered itself as a dispatcher, and its fire() function should never be called again. Then set up another breakpoint in gui2::event::sdl_event_handler::key_down() to see how the event handler still somehow manages to obtain a reference to the destroyed dispatcher.
Hmm, looks like the problem is different from what I originally thought.
This is the call stack when the command console receives the Enter key press:
> wesnoth.exe!gui2::dialogs::command_console::close() Line 86 C++
wesnoth.exe!gui2::dialogs::command_console::input_key_press_callback(13) Line 80 C++
wesnoth.exe!std::_Invoker_pmf_pointer::_Call<void (__thiscall gui2::dialogs::command_console::*)(int),gui2::dialogs::command_console * &,int>(0x00e7d4e0, 0xdddddddd, 13) Line 1342 C++
wesnoth.exe!std::invoke<void (__thiscall gui2::dialogs::command_console::*&)(int),gui2::dialogs::command_console * &,int>(0xdddddddd, 0xdddddddd, 13) Line 1443 C++
wesnoth.exe!std::_Invoke_ret<void (__thiscall gui2::dialogs::command_console::*&)(int),gui2::dialogs::command_console * &,int>({...}, 0xdddddddd, 0xdddddddd, 13) Line 1475 C++
wesnoth.exe!std::_Call_binder<std::_Unforced,0,1,void (__thiscall gui2::dialogs::command_console::*)(int),std::tuple<gui2::dialogs::command_console *,boost::arg<5> >,std::tuple<gui2::widget &,enum gui2::event::ui_event &&,bool &,bool &,int &&,enum SDL_Keymod &&,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &> >({...}, {...}, 0xdddddddd, {...}, {...}) Line 825 C++
wesnoth.exe!std::_Binder<std::_Unforced,void (__thiscall gui2::dialogs::command_console::*)(int),gui2::dialogs::command_console * const,boost::arg<5> const &>::operator()<gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>({...}, SDL_KEY_DOWN, false, false, 13, KMOD_NUM, {...}) Line 881 C++
wesnoth.exe!std::_Invoker_functor::_Call<std::_Binder<std::_Unforced,void (__thiscall gui2::dialogs::command_console::*)(int),gui2::dialogs::command_console * const,boost::arg<5> const &> &,gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>({...}, {...}, SDL_KEY_DOWN, false, false, 13, KMOD_NUM, {...}) Line 1377 C++
wesnoth.exe!std::invoke<std::_Binder<std::_Unforced,void (__thiscall gui2::dialogs::command_console::*)(int),gui2::dialogs::command_console * const,boost::arg<5> const &> &,gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>({...}, {...}, SDL_KEY_DOWN, false, false, 13, KMOD_NUM, {...}) Line 1443 C++
wesnoth.exe!std::_Invoke_ret<void,std::_Binder<std::_Unforced,void (__thiscall gui2::dialogs::command_console::*)(int),gui2::dialogs::command_console * const,boost::arg<5> const &> &,gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>({...}, {...}, {...}, SDL_KEY_DOWN, false, false, 13, KMOD_NUM, {...}) Line 1461 C++
wesnoth.exe!std::_Func_impl<std::_Binder<std::_Unforced,void (__thiscall gui2::dialogs::command_console::*)(int),gui2::dialogs::command_console * const,boost::arg<5> const &>,std::allocator<int>,void,gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>::_Do_call({...}, SDL_KEY_DOWN, false, false, 13, KMOD_NUM, {...}) Line 212 C++
wesnoth.exe!std::_Func_class<void,gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>::operator()({...}, SDL_KEY_DOWN, false, false, 13, KMOD_NUM, {...}) Line 280 C++
wesnoth.exe!gui2::event::implementation::fire_event<std::function<void __cdecl(gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &)>,int const &,enum SDL_Keymod const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>(SDL_KEY_DOWN, {...}, 0x2b7feb30, 0x2bf67040, 13, KMOD_NUM, {...}) Line 317 C++
wesnoth.exe!gui2::event::fire_event<std::function<void __cdecl(gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &)>,int const &,enum SDL_Keymod const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>(SDL_KEY_DOWN, 0x2b7feb38, 0x2bf67040, 13, KMOD_NUM, {...}) Line 386 C++
wesnoth.exe!gui2::event::dispatcher::fire(SDL_KEY_DOWN, {...}, 13, KMOD_NUM, {...}) Line 100 C++
wesnoth.exe!gui2::event::distributor::signal_handler_keyboard_internal<std::function<void __cdecl(gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &)>,int const &,enum SDL_Keymod const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>(SDL_KEY_DOWN, 13, KMOD_NUM, {...}) Line 660 C++
wesnoth.exe!gui2::event::distributor::signal_handler_sdl_key_down(13, KMOD_NUM, {...}) Line 713 C++
wesnoth.exe!std::_Invoker_pmf_pointer::_Call<void (__thiscall gui2::event::distributor::*)(int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &),gui2::event::distributor * &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>(0x00de01c2, 0xdddddddd, 13, KMOD_NUM, {...}) Line 1342 C++
wesnoth.exe!std::invoke<void (__thiscall gui2::event::distributor::*&)(int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &),gui2::event::distributor * &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>(0xdddddddd, 0xdddddddd, 13, KMOD_NUM, {...}) Line 1443 C++
wesnoth.exe!std::_Invoke_ret<void (__thiscall gui2::event::distributor::*&)(int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &),gui2::event::distributor * &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>({...}, 0xdddddddd, 0xdddddddd, 13, KMOD_NUM, {...}) Line 1475 C++
wesnoth.exe!std::_Call_binder<std::_Unforced,0,1,2,3,void (__thiscall gui2::event::distributor::*)(int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &),std::tuple<gui2::event::distributor *,boost::arg<5>,boost::arg<6>,boost::arg<7> >,std::tuple<gui2::widget &,enum gui2::event::ui_event &&,bool &,bool &,int &&,enum SDL_Keymod &&,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &> >({...}, {...}, 0xdddddddd, {...}, {...}) Line 825 C++
wesnoth.exe!std::_Binder<std::_Unforced,void (__thiscall gui2::event::distributor::*)(int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &),gui2::event::distributor * const,boost::arg<5> const &,boost::arg<6> const &,boost::arg<7> const &>::operator()<gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>({...}, SDL_KEY_DOWN, false, false, 13, KMOD_NUM, {...}) Line 881 C++
wesnoth.exe!std::_Invoker_functor::_Call<std::_Binder<std::_Unforced,void (__thiscall gui2::event::distributor::*)(int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &),gui2::event::distributor * const,boost::arg<5> const &,boost::arg<6> const &,boost::arg<7> const &> &,gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>({...}, {...}, SDL_KEY_DOWN, false, false, 13, KMOD_NUM, {...}) Line 1377 C++
wesnoth.exe!std::invoke<std::_Binder<std::_Unforced,void (__thiscall gui2::event::distributor::*)(int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &),gui2::event::distributor * const,boost::arg<5> const &,boost::arg<6> const &,boost::arg<7> const &> &,gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>({...}, {...}, SDL_KEY_DOWN, false, false, 13, KMOD_NUM, {...}) Line 1443 C++
wesnoth.exe!std::_Invoke_ret<void,std::_Binder<std::_Unforced,void (__thiscall gui2::event::distributor::*)(int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &),gui2::event::distributor * const,boost::arg<5> const &,boost::arg<6> const &,boost::arg<7> const &> &,gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>({...}, {...}, {...}, SDL_KEY_DOWN, false, false, 13, KMOD_NUM, {...}) Line 1461 C++
wesnoth.exe!std::_Func_impl<std::_Binder<std::_Unforced,void (__thiscall gui2::event::distributor::*)(int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &),gui2::event::distributor * const,boost::arg<5> const &,boost::arg<6> const &,boost::arg<7> const &>,std::allocator<int>,void,gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>::_Do_call({...}, SDL_KEY_DOWN, false, false, 13, KMOD_NUM, {...}) Line 212 C++
wesnoth.exe!std::_Func_class<void,gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>::operator()({...}, SDL_KEY_DOWN, false, false, 13, KMOD_NUM, {...}) Line 280 C++
wesnoth.exe!gui2::event::implementation::fire_event<std::function<void __cdecl(gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &)>,int const &,enum SDL_Keymod const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>(SDL_KEY_DOWN, {...}, 0x2b7feb30, 0x2b7feb30, 13, KMOD_NUM, {...}) Line 317 C++
wesnoth.exe!gui2::event::fire_event<std::function<void __cdecl(gui2::widget &,enum gui2::event::ui_event,bool &,bool &,int,enum SDL_Keymod,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &)>,int const &,enum SDL_Keymod const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &>(SDL_KEY_DOWN, 0x2b7feb38, 0x2b7feb30, 13, KMOD_NUM, {...}) Line 386 C++
wesnoth.exe!gui2::event::dispatcher::fire(SDL_KEY_DOWN, {...}, 13, KMOD_NUM, {...}) Line 100 C++
wesnoth.exe!gui2::event::sdl_event_handler::key_down(13, KMOD_NUM, {...}) Line 710 C++
wesnoth.exe!gui2::event::sdl_event_handler::key_down({...}) Line 663 C++
wesnoth.exe!gui2::event::sdl_event_handler::handle_event({...}) Line 373 C++
wesnoth.exe!events::run_event_loop() Line 616 C++
wesnoth.exe!controller_base::play_slice(true) Line 264 C++
wesnoth.exe!play_controller::play_slice_catch() Line 1058 C++
wesnoth.exe!playsingle_controller::play_human_turn() Line 466 C++
wesnoth.exe!playsingle_controller::play_side_impl() Line 387 C++
wesnoth.exe!play_controller::play_side() Line 1147 C++
wesnoth.exe!play_controller::play_turn() Line 1189 C++
wesnoth.exe!playsingle_controller::play_scenario_main_loop() Line 174 C++
wesnoth.exe!playsingle_controller::play_scenario({...}) Line 266 C++
wesnoth.exe!campaign_controller::playsingle_scenario({...}) Line 205 C++
wesnoth.exe!campaign_controller::play_game() Line 289 C++
wesnoth.exe!game_launcher::launch_game(RELOAD_DATA) Line 960 C++
wesnoth.exe!do_gameloop({...}) Line 887 C++
wesnoth.exe!SDL_main(1, 0x03f50018) Line 1063 C++
wesnoth.exe!main_utf8() Line 126 C
wesnoth.exe!main_getcmdline() Line 159 C
wesnoth.exe!WinMain() Line 203 C
wesnoth.exe!invoke_main() Line 99 C++
wesnoth.exe!__scrt_common_main_seh() Line 253 C++
wesnoth.exe!__scrt_common_main() Line 296 C++
wesnoth.exe!WinMainCRTStartup() Line 17 C++
kernel32.dll!@BaseThreadInitThunk@12() Unknown
ntdll.dll!__RtlUserThreadStart() Unknown
ntdll.dll!__RtlUserThreadStart@8() Unknown
Note that dispatcher::fire_event() is in the call stack twice. It called itself recursively.
The game survives the inner fire_event() just fine, but not the outer one.
Here is an experimental patch that fixes the crash: https://gist.github.com/jyrkive/ca524d26010cddf00076c2c18f829dff
@Vultraz can continue from there.
One of the bugs preventing migration of in-game + editor UI (Gui1 based) to GUI2 (aka modernization). Almost everything else is using gui2. Relevant is that modeless gui2 dialogs have various event problems when shown inside the in-game/editor map. We would like to be able to interact both with the map and with the dialog while useful dialogs are open (such as sidebar/menu implemented via GUI2 modeless dialogs).
