segfault on signal 2 exit with SDL music backend

[fabiangreffrath]

Set music backend to "native", kill the woof process by Pressing Ctrl-C in the console, close the "exit on signal 2" pop-up window -> segmentation fault. This does not happen with one of the other music backends, tested on Windows.

P_Init: Init Playloop state.
I_Init: Setting up machine state.
I_InitJoystick: Failed to open game controller.
I_InitSound: Configured audio device with 2048 samples/slice.
Precaching all sound effects...done.
I_InitMusic: Using SDL_mixer.
NET_Init: Init network subsystem.
D_CheckNetGame: Checking network game status.
startskill 2  deathmatch: 0  startmap: 1  startepisode: 1
player 1 of 1 (1 nodes)
S_Init: Setting up sound.
S_Init: default sfx volume 8
HU_Init: Setting up heads up display.
ST_Init: Init status bar.
I_SignalHandler: Exit on Signal 2
Segmentation fault

[rfomin]

It works on Windows right?

[fabiangreffrath]

No, I just found this on Windows.

[rfomin]

I can't reproduce it:

P_Init: Init Playloop state.
I_Init: Setting up machine state.
I_InitJoystick: Failed to open game controller.
I_InitSound: Configured audio device with 2048 samples/slice.
Precaching all sound effects...done.
I_InitMusic: Using SDL_mixer.
NET_Init: Init network subsystem.
D_CheckNetGame: Checking network game status.
startskill 3  deathmatch: 0  startmap: 1  startepisode: 1
player 1 of 1 (1 nodes)
S_Init: Setting up sound.
S_Init: default sfx volume 4
HU_Init: Setting up heads up display.
ST_Init: Init status bar.
I_SignalHandler: Exit on Signal 2

Do you run it in MSYS2 terminal?

[fabiangreffrath]

Yes, this is in MSYS2. Music backend must be set to "native", else this won't happen.

[fabiangreffrath]

It doesn't happen every time it seems.

[rfomin]

I can't get it so far.

[rfomin]

The problem with SDL_ShowMessageBox is that it always shows it in the background behind the game window on my system.

[fabiangreffrath]

I also suspect the culprit there.

--- a/Source/i_system.c
+++ b/Source/i_system.c
@@ -392,7 +392,7 @@ void I_Error(const char *error, ...) // killough 3/20/98: add const

     // If specified, don't show a GUI window for error messages when the
     // game exits with an error.
-    exit_gui_popup = !M_CheckParm("-nogui");
+    exit_gui_popup = !M_CheckParm("-nogui") && SDL_WasInit(0);

     // Pop up a GUI dialog box to show the error message, if the
     // game was not run from the console (and the user will

[fabiangreffrath]

I have another strange case: ./build/Source/woof.exe -record bla while recording the demo, Alt-Tab back to the terminal, press Ctrl-C while the popup window is up, wait for ~10 seconds the program exits with code 130

[rfomin]

while recording the demo, Alt-Tab back to the terminal, press Ctrl-C while the popup window is up, wait for ~10 seconds the program exits with code 130

This I can reproduce. I suggest removing message boxes from I_Error. People always ask what Signal 11 means :smile:

[kraflab]

I've been planning on "humanizing" the messages that pop up so that they are less cryptic for users but still give some info for them to include when reporting a crash. I think that's a better user experience than the program just exiting abruptly. But maybe not 🤔

[fabiangreffrath]

Some findings from today's research:

1. We show the I_Error pop-up window too early, when exiting the demo recording with Ctrl-C, the demo will still run in the background for some seconds.
2. The segfault during the atexit sequence seems to come from SDL_FreeSurface(sdlscreen) in I_QuitVideo(0) for whatever reason
3. Only the error message for the first call to I_Error() is ever reported, i.e. subsequent signal handlers etc. are never properly detected.

Ad 1, we should move the pop-up window into a separate function at register this with I_AtExitPrio(..., exit_priority_verylast). Ad 2, we could only call I_Quit() on regular exit, but this doesn't fix the actual cause for the segfault. Ad 3, We should print out each error message whenever I_Error() is called, but only pass the first one over to the pop-up window (or maybe concatenate them all together?).

[fabiangreffrath]

This I can reproduce. I suggest removing message boxes from I_Error. People always ask what Signal 11 means 😄

I'd like to keep the pop-up window for signal handling, because (a) it always happens unexpected and nothing is more frustrating than a game window closing in a split second without any further notice and (b) it never happens anyway (😁) and if it does, I want to know.

However, I agree that the actual error message could contain a bit more information than e.g. "signal 2", maybe something like "the game process has been interrupted" or "a segmentation fault occured, please inform the developers".

[rfomin]

2. The segfault during the atexit sequence seems to come from SDL_FreeSurface(sdlscreen) in I_QuitVideo(0) for whatever reason

I still can't reproduce it.

Ad 3, We should print out each error message whenever I_Error() is called, but only pass the first one over to the pop-up window (or maybe concatenate them all together?).

There isn't much reason to print since our Windows build doesn't have console output, I think we should concatenate.

[rfomin]

Interesting that Ctrl-C doesn't work at all in the release version, at least in cmd.exe.

[fabiangreffrath]

I don't even know if cmd.exe is able to pass signals to processes at all. It isn't a real shell after all.