From 0ff421d0017f3039e698c51452cd01658b7328fb Mon Sep 17 00:00:00 2001 From: James R Date: Wed, 11 Dec 2019 22:10:22 -0800 Subject: [PATCH 01/10] Move I_ShutdownConsole to I_ShutdownSystem --- src/sdl/i_system.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index 13fb25bea..03eeb634f 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -2169,7 +2169,6 @@ void I_Quit(void) if (quiting) goto death; SDLforceUngrabMouse(); quiting = SDL_FALSE; - I_ShutdownConsole(); M_SaveConfig(NULL); //save game config, cvars.. #ifndef NONET D_SaveBan(); // save the ban list @@ -2287,8 +2286,6 @@ void I_Error(const char *error, ...) I_OutputMsg("\nI_Error(): %s\n", buffer); // --- - I_ShutdownConsole(); - M_SaveConfig(NULL); // save game config, cvars.. #ifndef NONET D_SaveBan(); // save the ban list @@ -2388,6 +2385,8 @@ void I_ShutdownSystem(void) { INT32 c; + I_ShutdownConsole(); + for (c = MAX_QUIT_FUNCS-1; c >= 0; c--) if (quit_funcs[c]) (*quit_funcs[c])(); From 7c383e4a1f75b530d7759178c04d4ecfd7fc2dd1 Mon Sep 17 00:00:00 2001 From: James R Date: Wed, 11 Dec 2019 23:46:57 -0800 Subject: [PATCH 02/10] 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 d13ff9bc0..216b4fd90 100644 --- a/src/doomdef.h +++ b/src/doomdef.h @@ -125,6 +125,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 831acda32..131557ae9 100644 --- a/src/i_system.h +++ b/src/i_system.h @@ -84,6 +84,10 @@ ticcmd_t *I_BaseTiccmd2(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 029febc05..16d0a242c 100644 --- a/src/sdl/i_main.c +++ b/src/sdl/i_main.c @@ -28,6 +28,11 @@ #include "time.h" // For log timestamps +#ifdef NEWSIGNALHANDLER +#include +#include +#endif + #ifdef HAVE_SDL #ifdef HAVE_TTF @@ -181,6 +186,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 SRB2...\n"); D_SRB2Main(); diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index 03eeb634f..e58691de6 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -229,13 +229,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) { @@ -261,8 +259,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); @@ -270,11 +281,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) { @@ -664,10 +683,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 6dcdb8d95179cc6efd67c70ec63232816e18d10c Mon Sep 17 00:00:00 2001 From: James R Date: Thu, 12 Dec 2019 15:07:59 -0800 Subject: [PATCH 03/10] 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 216b4fd90..d13ff9bc0 100644 --- a/src/doomdef.h +++ b/src/doomdef.h @@ -125,10 +125,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 131557ae9..831acda32 100644 --- a/src/i_system.h +++ b/src/i_system.h @@ -84,10 +84,6 @@ ticcmd_t *I_BaseTiccmd2(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 16d0a242c..69e49b4a7 100644 --- a/src/sdl/i_main.c +++ b/src/sdl/i_main.c @@ -28,11 +28,6 @@ #include "time.h" // For log timestamps -#ifdef NEWSIGNALHANDLER -#include -#include -#endif - #ifdef HAVE_SDL #ifdef HAVE_TTF @@ -187,52 +182,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 SRB2...\n"); D_SRB2Main(); diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index e58691de6..9245673cc 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 @@ -229,7 +235,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; @@ -2160,6 +2166,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; @@ -2167,6 +2220,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 f34886850ff59c51b2b70afeee755bf4e378a500 Mon Sep 17 00:00:00 2001 From: James R Date: Thu, 12 Dec 2019 21:20:51 -0800 Subject: [PATCH 04/10] 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 9245673cc..c3b469bca 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -2182,7 +2182,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 72ee91702cf7cec3a2076cb73e19835104dcc41d Mon Sep 17 00:00:00 2001 From: James R Date: Thu, 12 Dec 2019 21:21:25 -0800 Subject: [PATCH 05/10] 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 c3b469bca..7d1405268 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -2169,9 +2169,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( @@ -2184,6 +2188,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 35053adfb2fdb5cf224b5170dd9fa1ee57413723 Mon Sep 17 00:00:00 2001 From: James R Date: Thu, 12 Dec 2019 21:30:35 -0800 Subject: [PATCH 06/10] 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 397406293..00647048d 100644 --- a/src/d_main.c +++ b/src/d_main.c @@ -562,9 +562,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 831acda32..2532ba0ee 100644 --- a/src/i_system.h +++ b/src/i_system.h @@ -184,10 +184,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 7d1405268..95745497c 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -675,7 +675,7 @@ static inline void I_ShutdownConsole(void){} // // StartupKeyboard // -void I_StartupKeyboard (void) +void I_RegisterSignals (void) { #ifdef SIGINT signal(SIGINT , quit_handler); @@ -2228,6 +2228,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 838850ffc1a240a1a1381f4b297995d49dcd0ddc Mon Sep 17 00:00:00 2001 From: James R Date: Thu, 12 Dec 2019 21:57:54 -0800 Subject: [PATCH 07/10] Fix signal handler setup error reporting --- src/sdl/i_system.c | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index 95745497c..4bed8e7f2 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -2167,6 +2167,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; @@ -2178,10 +2198,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; @@ -2189,10 +2206,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 { @@ -2211,7 +2225,7 @@ static void I_Fork(void) status = WEXITSTATUS (status); } - I_ShutdownSystem(); + I_ShutdownConsole(); exit(status); } } @@ -2468,8 +2482,6 @@ void I_ShutdownSystem(void) { INT32 c; - I_ShutdownConsole(); - for (c = MAX_QUIT_FUNCS-1; c >= 0; c--) if (quit_funcs[c]) (*quit_funcs[c])(); From 8bea7f6dbc87f29239286644627167cb516bc9d4 Mon Sep 17 00:00:00 2001 From: James R Date: Thu, 12 Dec 2019 22:01:16 -0800 Subject: [PATCH 08/10] 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 4bed8e7f2..a28fadc02 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -282,10 +282,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 553ad46c74cd99b77300589ec9a592370b80441e Mon Sep 17 00:00:00 2001 From: James R Date: Fri, 13 Dec 2019 15:04:31 -0800 Subject: [PATCH 09/10] 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 a28fadc02..effdec8e2 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -2482,6 +2482,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 8a23ff0bc857d93eb9c7218f6a957fc8f4db6335 Mon Sep 17 00:00:00 2001 From: James R Date: Fri, 13 Dec 2019 16:51:49 -0800 Subject: [PATCH 10/10] Handle log file in parent properly --- src/doomdef.h | 1 + src/sdl/i_main.c | 12 +++++++----- src/sdl/i_system.c | 15 ++++++++++++++- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/doomdef.h b/src/doomdef.h index d13ff9bc0..b2be062c3 100644 --- a/src/doomdef.h +++ b/src/doomdef.h @@ -127,6 +127,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 69e49b4a7..d0830396f 100644 --- a/src/sdl/i_main.c +++ b/src/sdl/i_main.c @@ -47,6 +47,7 @@ extern int SDL_main(int argc, char *argv[]); #ifdef LOGMESSAGES FILE *logstream = NULL; +char logfilename[1024]; #endif #ifndef DOXYGEN @@ -116,7 +117,6 @@ int main(int argc, char **argv) #endif { const char *logdir = NULL; - char logfile[MAX_WADPATH]; myargc = argc; myargv = argv; /// \todo pull out path to exe from this string @@ -141,7 +141,7 @@ int main(int argc, char **argv) timeinfo = localtime(&my_time); strftime(buf, 26, "%Y-%m-%d %H-%M-%S", timeinfo); - strcpy(logfile, va("log-%s.txt", buf)); + strcpy(logfilename, va("log-%s.txt", buf)); #ifdef DEFAULTDIR if (logdir) @@ -149,14 +149,16 @@ int main(int argc, char **argv) // Create dirs here because D_SRB2Main() is too late. I_mkdir(va("%s%s"DEFAULTDIR, logdir, PATHSEP), 0755); I_mkdir(va("%s%s"DEFAULTDIR"%slogs",logdir, PATHSEP, PATHSEP), 0755); - logstream = fopen(va("%s%s"DEFAULTDIR"%slogs%s%s",logdir, PATHSEP, PATHSEP, PATHSEP, logfile), "wt"); + strcpy(logfilename, va("%s%s"DEFAULTDIR"%slogs%s%s",logdir, PATHSEP, PATHSEP, PATHSEP, logfilename)); } else #endif { I_mkdir("."PATHSEP"logs"PATHSEP, 0755); - logstream = fopen(va("."PATHSEP"logs"PATHSEP"%s", logfile), "wt"); + strcpy(logfilename, va("."PATHSEP"logs"PATHSEP"%s", logfilename)); } + + logstream = fopen(logfilename, "wt"); } #endif @@ -187,7 +189,7 @@ int main(int argc, char **argv) D_SRB2Main(); #ifdef LOGMESSAGES if (!M_CheckParm("-nolog")) - CONS_Printf("Logfile: %s\n", logfile); + CONS_Printf("Logfile: %s\n", logfilename); #endif CONS_Printf("Entering main game loop...\n"); // never return diff --git a/src/sdl/i_system.c b/src/sdl/i_system.c index effdec8e2..a5e536bf5 100644 --- a/src/sdl/i_system.c +++ b/src/sdl/i_system.c @@ -2192,6 +2192,7 @@ static void I_Fork(void) int child; int status; int signum; + int c; child = fork(); @@ -2203,7 +2204,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()");