From 2b837726ebcb84b67408aa033887e2a115721237 Mon Sep 17 00:00:00 2001 From: James R Date: Wed, 11 Dec 2019 23:46:57 -0800 Subject: [PATCH 1/9] nix: Fork before game code and wait to catch signals and coredumps Ditched signal_handler to avoid worrying about async-signal-safe functions. D_QuitNetGame is not called, so players whose programs are interrupted by a signal will time out from the server. Because the game runs in a child process, the window can close before the "Signal Caught" text box appears. "(core dumped)" is also included in the message if core dumping could be determined. --- src/doomdef.h | 4 ++++ src/i_system.h | 4 ++++ src/sdl/i_main.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++ src/sdl/i_system.c | 33 +++++++++++++++++++++++------ 4 files changed, 87 insertions(+), 6 deletions(-) diff --git a/src/doomdef.h b/src/doomdef.h index 24b52e8d..94bd71d3 100644 --- a/src/doomdef.h +++ b/src/doomdef.h @@ -136,6 +136,10 @@ #define LOGMESSAGES // write message in log.txt #endif +#if (defined (__unix__) && !defined (_MSDOS)) || defined (UNIXCOMMON) +#define NEWSIGNALHANDLER +#endif + #ifdef LOGMESSAGES extern FILE *logstream; #endif diff --git a/src/i_system.h b/src/i_system.h index d1708fe5..e8c516fe 100644 --- a/src/i_system.h +++ b/src/i_system.h @@ -92,6 +92,10 @@ ticcmd_t *I_BaseTiccmd4(void); */ void I_Quit(void) FUNCNORETURN; +/** \brief Print a message and text box about a signal that was raised. +*/ +void I_ReportSignal(int num, int coredumped); + typedef enum { EvilForce = -1, diff --git a/src/sdl/i_main.c b/src/sdl/i_main.c index 41a9d7cd..b8161abf 100644 --- a/src/sdl/i_main.c +++ b/src/sdl/i_main.c @@ -26,6 +26,11 @@ #include #endif +#ifdef NEWSIGNALHANDLER +#include +#include +#endif + #ifdef HAVE_SDL #ifdef HAVE_TTF @@ -157,6 +162,53 @@ int main(int argc, char **argv) #endif MakeCodeWritable(); #endif + +#ifdef NEWSIGNALHANDLER + switch (fork()) + { + case -1: + I_Error( + "Error setting up signal reporting: fork(): %s\n", + strerror(errno) + ); + break; + case 0: + break; + default: + { + int status; + int signum; + if (wait(&status) == -1) + { + I_Error( + "Error setting up signal reporting: fork(): %s\n", + strerror(errno) + ); + } + else + { + if (WIFSIGNALED (status)) + { + signum = WTERMSIG (status); +#ifdef WCOREDUMP + I_ReportSignal(signum, WCOREDUMP (status)); +#else + I_ReportSignal(signum, 0); +#endif + status = 128 + signum; + } + else if (WIFEXITED (status)) + { + status = WEXITSTATUS (status); + } + + I_ShutdownSystem(); + exit(status); + } + } + } +#endif/*NEWSIGNALHANDLER*/ + // startup SRB2 CONS_Printf("Setting up SRB2Kart...\n"); D_SRB2Main(); diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index c0fca64d..db35b0f7 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -246,13 +246,11 @@ SDL_bool framebuffer = SDL_FALSE; UINT8 keyboard_started = false; -FUNCNORETURN static ATTRNORETURN void signal_handler(INT32 num) +void I_ReportSignal(int num, int coredumped) { //static char msg[] = "oh no! back to reality!\r\n"; const char * sigmsg; - char sigdef[32]; - - D_QuitNetGame(); // Fix server freezes + char msg[128]; switch (num) { @@ -278,8 +276,21 @@ FUNCNORETURN static ATTRNORETURN void signal_handler(INT32 num) sigmsg = "SIGABRT - abnormal termination triggered by abort call"; break; default: - sprintf(sigdef,"signal number %d", num); - sigmsg = sigdef; + sprintf(msg,"signal number %d", num); + if (coredumped) + sigmsg = 0; + else + sigmsg = msg; + } + + if (coredumped) + { + if (sigmsg) + sprintf(msg, "%s (core dumped)", sigmsg); + else + strcat(msg, " (core dumped)"); + + sigmsg = msg; } I_OutputMsg("\nsignal_handler() error: %s\n", sigmsg); @@ -287,11 +298,19 @@ FUNCNORETURN static ATTRNORETURN void signal_handler(INT32 num) SDL_ShowSimpleMessageBox(SDL_MESSAGEBOX_ERROR, "Signal caught", sigmsg, NULL); +} + +#ifndef NEWSIGNALHANDLER +FUNCNORETURN static ATTRNORETURN void signal_handler(INT32 num) +{ + D_QuitNetGame(); // Fix server freezes + I_ReportSignal(num, 0); I_ShutdownSystem(); signal(num, SIG_DFL); //default signal action raise(num); I_Quit(); } +#endif FUNCNORETURN static ATTRNORETURN void quit_handler(int num) { @@ -681,10 +700,12 @@ void I_StartupKeyboard (void) // If these defines don't exist, // then compilation would have failed above us... +#ifndef NEWSIGNALHANDLER signal(SIGILL , signal_handler); signal(SIGSEGV , signal_handler); signal(SIGABRT , signal_handler); signal(SIGFPE , signal_handler); +#endif } // From 9efe4d844594fa4dd596432e0bf116b6cd7e9948 Mon Sep 17 00:00:00 2001 From: James R Date: Thu, 12 Dec 2019 15:07:59 -0800 Subject: [PATCH 2/9] Move everything to i_system.c This also simplifies things; SDL isn't initialized in the parent process. --- src/doomdef.h | 4 ---- src/i_system.h | 4 ---- src/sdl/i_main.c | 51 ---------------------------------------- src/sdl/i_system.c | 58 +++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 57 insertions(+), 60 deletions(-) diff --git a/src/doomdef.h b/src/doomdef.h index 94bd71d3..24b52e8d 100644 --- a/src/doomdef.h +++ b/src/doomdef.h @@ -136,10 +136,6 @@ #define LOGMESSAGES // write message in log.txt #endif -#if (defined (__unix__) && !defined (_MSDOS)) || defined (UNIXCOMMON) -#define NEWSIGNALHANDLER -#endif - #ifdef LOGMESSAGES extern FILE *logstream; #endif diff --git a/src/i_system.h b/src/i_system.h index e8c516fe..d1708fe5 100644 --- a/src/i_system.h +++ b/src/i_system.h @@ -92,10 +92,6 @@ ticcmd_t *I_BaseTiccmd4(void); */ void I_Quit(void) FUNCNORETURN; -/** \brief Print a message and text box about a signal that was raised. -*/ -void I_ReportSignal(int num, int coredumped); - typedef enum { EvilForce = -1, diff --git a/src/sdl/i_main.c b/src/sdl/i_main.c index b8161abf..e7d87631 100644 --- a/src/sdl/i_main.c +++ b/src/sdl/i_main.c @@ -26,11 +26,6 @@ #include #endif -#ifdef NEWSIGNALHANDLER -#include -#include -#endif - #ifdef HAVE_SDL #ifdef HAVE_TTF @@ -163,52 +158,6 @@ int main(int argc, char **argv) MakeCodeWritable(); #endif -#ifdef NEWSIGNALHANDLER - switch (fork()) - { - case -1: - I_Error( - "Error setting up signal reporting: fork(): %s\n", - strerror(errno) - ); - break; - case 0: - break; - default: - { - int status; - int signum; - if (wait(&status) == -1) - { - I_Error( - "Error setting up signal reporting: fork(): %s\n", - strerror(errno) - ); - } - else - { - if (WIFSIGNALED (status)) - { - signum = WTERMSIG (status); -#ifdef WCOREDUMP - I_ReportSignal(signum, WCOREDUMP (status)); -#else - I_ReportSignal(signum, 0); -#endif - status = 128 + signum; - } - else if (WIFEXITED (status)) - { - status = WEXITSTATUS (status); - } - - I_ShutdownSystem(); - exit(status); - } - } - } -#endif/*NEWSIGNALHANDLER*/ - // startup SRB2 CONS_Printf("Setting up SRB2Kart...\n"); D_SRB2Main(); diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index db35b0f7..4cbf7626 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -102,6 +102,12 @@ typedef LPVOID (WINAPI *p_MapViewOfFile) (HANDLE, DWORD, DWORD, DWORD, SIZE_T); #endif #endif +#if (defined (__unix__) && !defined (_MSDOS)) || defined (UNIXCOMMON) +#include +#include +#define NEWSIGNALHANDLER +#endif + #ifndef NOMUMBLE #ifdef __linux__ // need -lrt #include @@ -246,7 +252,7 @@ SDL_bool framebuffer = SDL_FALSE; UINT8 keyboard_started = false; -void I_ReportSignal(int num, int coredumped) +static void I_ReportSignal(int num, int coredumped) { //static char msg[] = "oh no! back to reality!\r\n"; const char * sigmsg; @@ -3043,6 +3049,53 @@ void I_Sleep(void) SDL_Delay(cv_sleep.value); } +#ifdef NEWSIGNALHANDLER +static void I_Fork(void) +{ + int status; + int signum; + switch (fork()) + { + case -1: + I_Error( + "Error setting up signal reporting: fork(): %s\n", + strerror(errno) + ); + break; + case 0: + break; + default: + if (wait(&status)) + { + I_Error( + "Error setting up signal reporting: fork(): %s\n", + strerror(errno) + ); + } + else + { + if (WIFSIGNALED (status)) + { + signum = WTERMSIG (status); +#ifdef WCOREDUMP + I_ReportSignal(signum, WCOREDUMP (status)); +#else + I_ReportSignal(signum, 0); +#endif + status = 128 + signum; + } + else if (WIFEXITED (status)) + { + status = WEXITSTATUS (status); + } + + I_ShutdownSystem(); + exit(status); + } + } +} +#endif/*NEWSIGNALHANDLER*/ + INT32 I_StartupSystem(void) { SDL_version SDLcompiled; @@ -3050,6 +3103,9 @@ INT32 I_StartupSystem(void) SDL_VERSION(&SDLcompiled) SDL_GetVersion(&SDLlinked); I_StartupConsole(); +#ifdef NEWSIGNALHANDLER + I_Fork(); +#endif I_OutputMsg("Compiled for SDL version: %d.%d.%d\n", SDLcompiled.major, SDLcompiled.minor, SDLcompiled.patch); I_OutputMsg("Linked with SDL version: %d.%d.%d\n", From c9830e5ab1dd634c23006e4633ddbb86dea35e73 Mon Sep 17 00:00:00 2001 From: James R Date: Thu, 12 Dec 2019 21:20:51 -0800 Subject: [PATCH 3/9] Fix idiot mistake --- src/sdl/i_system.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index 4cbf7626..3f64a3d4 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -3065,7 +3065,7 @@ static void I_Fork(void) case 0: break; default: - if (wait(&status)) + if (wait(&status) == -1) { I_Error( "Error setting up signal reporting: fork(): %s\n", From a64dbe101664a19c274fd9bc1cbbed6638b17e26 Mon Sep 17 00:00:00 2001 From: James R Date: Thu, 12 Dec 2019 21:21:25 -0800 Subject: [PATCH 4/9] Kill child when wait fails, so I_Error exits both --- src/sdl/i_system.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index 3f64a3d4..aa74d7d9 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -3052,9 +3052,13 @@ void I_Sleep(void) #ifdef NEWSIGNALHANDLER static void I_Fork(void) { + int child; int status; int signum; - switch (fork()) + + child = fork(); + + switch (child) { case -1: I_Error( @@ -3067,6 +3071,7 @@ static void I_Fork(void) default: if (wait(&status) == -1) { + kill(child, SIGKILL); I_Error( "Error setting up signal reporting: fork(): %s\n", strerror(errno) From b83f41e089bbd06c9ac1c2c177c552b0647cb83e Mon Sep 17 00:00:00 2001 From: James R Date: Thu, 12 Dec 2019 21:30:35 -0800 Subject: [PATCH 5/9] Rename I_StartupKeyboard to I_RegisterSignals and call it in a sane place --- src/d_main.c | 3 --- src/i_system.h | 4 ---- src/sdl/i_system.c | 3 ++- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/d_main.c b/src/d_main.c index 467976c1..e3252e88 100644 --- a/src/d_main.c +++ b/src/d_main.c @@ -613,9 +613,6 @@ void D_SRB2Loop(void) // Pushing of + parameters is now done back in D_SRB2Main, not here. - CONS_Printf("I_StartupKeyboard()...\n"); - I_StartupKeyboard(); - #ifdef _WINDOWS CONS_Printf("I_StartupMouse()...\n"); I_DoStartupMouse(); diff --git a/src/i_system.h b/src/i_system.h index d1708fe5..3e589c69 100644 --- a/src/i_system.h +++ b/src/i_system.h @@ -226,10 +226,6 @@ void I_StartupMouse(void); */ void I_StartupMouse2(void); -/** \brief keyboard startup, shutdown, handler -*/ -void I_StartupKeyboard(void); - /** \brief setup timer irq and user timer routine. */ void I_StartupTimer(void); diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index aa74d7d9..d8404e24 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -692,7 +692,7 @@ static inline void I_ShutdownConsole(void){} // // StartupKeyboard // -void I_StartupKeyboard (void) +void I_RegisterSignals (void) { #ifdef SIGINT signal(SIGINT , quit_handler); @@ -3111,6 +3111,7 @@ INT32 I_StartupSystem(void) #ifdef NEWSIGNALHANDLER I_Fork(); #endif + I_RegisterSignals(); I_OutputMsg("Compiled for SDL version: %d.%d.%d\n", SDLcompiled.major, SDLcompiled.minor, SDLcompiled.patch); I_OutputMsg("Linked with SDL version: %d.%d.%d\n", From a0d6dc30cbe38e4b3e2a3bb4b8cb6f375eb00c64 Mon Sep 17 00:00:00 2001 From: James R Date: Thu, 12 Dec 2019 21:57:54 -0800 Subject: [PATCH 6/9] Fix signal handler setup error reporting --- src/sdl/i_system.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index d8404e24..1b9cc2ed 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -3050,6 +3050,26 @@ void I_Sleep(void) } #ifdef NEWSIGNALHANDLER +static void newsignalhandler_Warn(const char *pr) +{ + char text[128]; + + snprintf(text, sizeof text, + "Error while setting up signal reporting: %s: %s", + pr, + strerror(errno) + ); + + I_OutputMsg("%s\n", text); + + SDL_ShowSimpleMessageBox(SDL_MESSAGEBOX_ERROR, + "Startup error", + text, NULL); + + I_ShutdownConsole(); + exit(-1); +} + static void I_Fork(void) { int child; @@ -3061,10 +3081,7 @@ static void I_Fork(void) switch (child) { case -1: - I_Error( - "Error setting up signal reporting: fork(): %s\n", - strerror(errno) - ); + newsignalhandler_Warn("fork()"); break; case 0: break; @@ -3072,10 +3089,7 @@ static void I_Fork(void) if (wait(&status) == -1) { kill(child, SIGKILL); - I_Error( - "Error setting up signal reporting: fork(): %s\n", - strerror(errno) - ); + newsignalhandler_Warn("wait()"); } else { @@ -3094,7 +3108,7 @@ static void I_Fork(void) status = WEXITSTATUS (status); } - I_ShutdownSystem(); + I_ShutdownConsole(); exit(status); } } From d0c41a8d55a9ddab2487062fcb290230246be45e Mon Sep 17 00:00:00 2001 From: James R Date: Thu, 12 Dec 2019 22:01:16 -0800 Subject: [PATCH 7/9] Rename signal caught message to be more obvious --- src/sdl/i_system.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index 1b9cc2ed..d4297c14 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -299,10 +299,10 @@ static void I_ReportSignal(int num, int coredumped) sigmsg = msg; } - I_OutputMsg("\nsignal_handler() error: %s\n", sigmsg); + I_OutputMsg("\nProcess killed by signal: %s\n\n", sigmsg); SDL_ShowSimpleMessageBox(SDL_MESSAGEBOX_ERROR, - "Signal caught", + "Process killed by signal", sigmsg, NULL); } From f460e83846bdcfa4f52fe60591f79c28c151102f Mon Sep 17 00:00:00 2001 From: James R Date: Fri, 13 Dec 2019 15:04:31 -0800 Subject: [PATCH 8/9] Add this back for Windoodoo because I'm an idiot --- src/sdl/i_system.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index d4297c14..912e3fee 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -3368,6 +3368,10 @@ void I_ShutdownSystem(void) { INT32 c; +#ifndef NEWSIGNALHANDLER + I_ShutdownConsole(); +#endif + for (c = MAX_QUIT_FUNCS-1; c >= 0; c--) if (quit_funcs[c]) (*quit_funcs[c])(); From 2a016332c469aa173d54a4d079f5802a3aed8ff6 Mon Sep 17 00:00:00 2001 From: James R Date: Fri, 13 Dec 2019 16:51:49 -0800 Subject: [PATCH 9/9] Handle log file in parent properly --- src/doomdef.h | 1 + src/sdl/i_main.c | 7 +++++-- src/sdl/i_system.c | 15 ++++++++++++++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/doomdef.h b/src/doomdef.h index 24b52e8d..a0896094 100644 --- a/src/doomdef.h +++ b/src/doomdef.h @@ -138,6 +138,7 @@ #ifdef LOGMESSAGES extern FILE *logstream; +extern char logfilename[1024]; #endif //#define DEVELOP // Disable this for release builds to remove excessive cheat commands and enable MD5 checking and stuff, all in one go. :3 diff --git a/src/sdl/i_main.c b/src/sdl/i_main.c index e7d87631..41874e9f 100644 --- a/src/sdl/i_main.c +++ b/src/sdl/i_main.c @@ -45,6 +45,7 @@ extern int SDL_main(int argc, char *argv[]); #ifdef LOGMESSAGES FILE *logstream = NULL; +char logfilename[1024]; #endif #ifndef DOXYGEN @@ -130,10 +131,12 @@ int main(int argc, char **argv) #ifdef LOGMESSAGES #ifdef DEFAULTDIR if (logdir) - logstream = fopen(va("%s/"DEFAULTDIR"/log.txt",logdir), "wt"); + strcpy(logfilename, va("%s/"DEFAULTDIR"/log.txt",logdir)); else #endif - logstream = fopen("./log.txt", "wt"); + strcpy(logfilename, "./log.txt"); + + logstream = fopen(logfilename, "wt"); #endif //I_OutputMsg("I_StartupSystem() ...\n"); diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index 912e3fee..37d6dd79 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -3075,6 +3075,7 @@ static void I_Fork(void) int child; int status; int signum; + int c; child = fork(); @@ -3086,7 +3087,19 @@ static void I_Fork(void) case 0: break; default: - if (wait(&status) == -1) + if (logstream) + fclose(logstream);/* the child has this */ + + c = wait(&status); + +#ifdef LOGMESSAGES + /* By the way, exit closes files. */ + logstream = fopen(logfilename, "at"); +#else + logstream = 0; +#endif + + if (c == -1) { kill(child, SIGKILL); newsignalhandler_Warn("wait()");