summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Deseyn <tom.deseyn@gmail.com>2019-01-15 21:43:08 +0100
committerJan Kotas <jkotas@microsoft.com>2019-01-15 12:43:08 -0800
commit3f0903598d0e269e6aaf8e237b47269abe4ab7e8 (patch)
treeaff29f9a428caf4b1591987dff07fd146d15909b
parent86b4ac59ef9d19e8f635086ea6a20a291c1dd26a (diff)
downloadcoreclr-3f0903598d0e269e6aaf8e237b47269abe4ab7e8.tar.gz
coreclr-3f0903598d0e269e6aaf8e237b47269abe4ab7e8.tar.bz2
coreclr-3f0903598d0e269e6aaf8e237b47269abe4ab7e8.zip
On SIGTERM default to a non-zero exit code (#21300)
* On SIGTERM default to a non-zero exit code * Fix Windows builds * Improve SIG_DFL/SIG_IGN handling * Remove PAL_GetTerminationExitCode * Use sa_handler/sa_sigaction based on SA_SIGINFO; remove HAVE_SIGINFO_T. * configure.cmake: remove siginfo_t check * Move restore_signal_and_resend so OSX can use it; add function documentation * Fix OSX build: include pal/process.h for gPID * Check SIG_IGN and SIG_DFL against sa_handler * Don't use sa_handler when SA_SIGINFO is set * Fix equality check * Swap order of checking SA_SIGINFO and SIG_IGN/SIG_DFL
-rw-r--r--src/pal/inc/pal.h2
-rw-r--r--src/pal/src/config.h.in1
-rw-r--r--src/pal/src/configure.cmake1
-rw-r--r--src/pal/src/exception/signal.cpp191
-rw-r--r--src/pal/src/synchmgr/synchmanager.cpp6
-rw-r--r--src/vm/exceptionhandling.cpp20
6 files changed, 130 insertions, 91 deletions
diff --git a/src/pal/inc/pal.h b/src/pal/inc/pal.h
index d49cb13589..76e04cdbdb 100644
--- a/src/pal/inc/pal.h
+++ b/src/pal/inc/pal.h
@@ -5057,7 +5057,7 @@ public:
typedef BOOL (*PHARDWARE_EXCEPTION_HANDLER)(PAL_SEHException* ex);
typedef BOOL (*PHARDWARE_EXCEPTION_SAFETY_CHECK_FUNCTION)(PCONTEXT contextRecord, PEXCEPTION_RECORD exceptionRecord);
-typedef VOID (*PTERMINATION_REQUEST_HANDLER)();
+typedef VOID (*PTERMINATION_REQUEST_HANDLER)(int terminationExitCode);
typedef DWORD (*PGET_GCMARKER_EXCEPTION_CODE)(LPVOID ip);
PALIMPORT
diff --git a/src/pal/src/config.h.in b/src/pal/src/config.h.in
index 1ed071e33f..d0e14df13a 100644
--- a/src/pal/src/config.h.in
+++ b/src/pal/src/config.h.in
@@ -78,7 +78,6 @@
#cmakedefine01 HAVE_GREGSET_T
#cmakedefine01 HAVE___GREGSET_T
#cmakedefine01 HAVE_FPSTATE_GLIBC_RESERVED1
-#cmakedefine01 HAVE_SIGINFO_T
#cmakedefine01 HAVE_UCONTEXT_T
#cmakedefine01 HAVE_PTHREAD_RWLOCK_T
#cmakedefine01 HAVE_PRWATCH_T
diff --git a/src/pal/src/configure.cmake b/src/pal/src/configure.cmake
index d105d851e3..c8b6f3bd6e 100644
--- a/src/pal/src/configure.cmake
+++ b/src/pal/src/configure.cmake
@@ -128,7 +128,6 @@ set(CMAKE_EXTRA_INCLUDE_FILES asm/ptrace.h)
check_type_size("struct pt_regs" PT_REGS)
set(CMAKE_EXTRA_INCLUDE_FILES)
set(CMAKE_EXTRA_INCLUDE_FILES signal.h)
-check_type_size(siginfo_t SIGINFO_T)
set(CMAKE_EXTRA_INCLUDE_FILES)
set(CMAKE_EXTRA_INCLUDE_FILES ucontext.h)
check_type_size(ucontext_t UCONTEXT_T)
diff --git a/src/pal/src/exception/signal.cpp b/src/pal/src/exception/signal.cpp
index fe15a70f83..df5a5f22a4 100644
--- a/src/pal/src/exception/signal.cpp
+++ b/src/pal/src/exception/signal.cpp
@@ -23,6 +23,7 @@ SET_DEFAULT_DEBUG_CHANNEL(EXCEPT); // some headers have code with asserts, so do
#include "pal/corunix.hpp"
#include "pal/handleapi.hpp"
+#include "pal/process.h"
#include "pal/thread.hpp"
#include "pal/threadinfo.hpp"
#include "pal/threadsusp.hpp"
@@ -36,7 +37,6 @@ SET_DEFAULT_DEBUG_CHANNEL(EXCEPT); // some headers have code with asserts, so do
#if !HAVE_MACH_EXCEPTIONS
#include "pal/init.h"
-#include "pal/process.h"
#include "pal/debug.h"
#include "pal/virtual.h"
#include "pal/utils.h"
@@ -62,12 +62,6 @@ using namespace CorUnix;
/* local type definitions *****************************************************/
-#if !HAVE_SIGINFO_T
-/* This allows us to compile on platforms that don't have siginfo_t.
- * Exceptions will work poorly on those platforms. */
-#warning Exceptions will work poorly on this platform
-typedef void *siginfo_t;
-#endif /* !HAVE_SIGINFO_T */
typedef void (*SIGFUNC)(int, siginfo_t *, void *);
/* internal function declarations *********************************************/
@@ -91,6 +85,7 @@ static void inject_activation_handler(int code, siginfo_t *siginfo, void *contex
static void handle_signal(int signal_id, SIGFUNC sigfunc, struct sigaction *previousAction, int additionalFlags = 0, bool skipIgnored = false);
static void restore_signal(int signal_id, struct sigaction *previousAction);
+static void restore_signal_and_resend(int code, struct sigaction* action);
/* internal data declarations *********************************************/
@@ -242,6 +237,68 @@ void SEHCleanupSignals()
#if !HAVE_MACH_EXCEPTIONS
/*++
Function :
+ invoke_previous_action
+
+ synchronously invokes the previous action or aborts when that is not possible
+
+Parameters :
+ action : previous sigaction struct
+ code : signal code
+ siginfo : signal siginfo
+ context : signal context
+ signalRestarts: BOOL state : TRUE if the process will be signalled again
+
+ (no return value)
+--*/
+static void invoke_previous_action(struct sigaction* action, int code, siginfo_t *siginfo, void *context, bool signalRestarts = true)
+{
+ _ASSERTE(action != NULL);
+
+ if (action->sa_flags & SA_SIGINFO)
+ {
+ // Directly call the previous handler.
+ _ASSERTE(action->sa_sigaction != NULL);
+ action->sa_sigaction(code, siginfo, context);
+ }
+ else
+ {
+ if (action->sa_handler == SIG_IGN)
+ {
+ if (signalRestarts)
+ {
+ // This signal mustn't be ignored because it will be restarted.
+ PROCAbort();
+ }
+ return;
+ }
+ else if (action->sa_handler == SIG_DFL)
+ {
+ if (signalRestarts)
+ {
+ // Restore the original and restart h/w exception.
+ restore_signal(code, action);
+ }
+ else
+ {
+ // We can't invoke the original handler because returning from the
+ // handler doesn't restart the exception.
+ PROCAbort();
+ }
+ }
+ else
+ {
+ // Directly call the previous handler.
+ _ASSERTE(action->sa_handler != NULL);
+ action->sa_handler(code);
+ }
+ }
+
+ PROCNotifyProcessShutdown();
+ PROCCreateCrashDumpIfEnabled();
+}
+
+/*++
+Function :
sigill_handler
handle SIGILL signal (EXCEPTION_ILLEGAL_INSTRUCTION, others?)
@@ -261,18 +318,7 @@ static void sigill_handler(int code, siginfo_t *siginfo, void *context)
}
}
- if (g_previous_sigill.sa_sigaction != NULL)
- {
- g_previous_sigill.sa_sigaction(code, siginfo, context);
- }
- else
- {
- // Restore the original or default handler and restart h/w exception
- restore_signal(code, &g_previous_sigill);
- }
-
- PROCNotifyProcessShutdown();
- PROCCreateCrashDumpIfEnabled();
+ invoke_previous_action(&g_previous_sigill, code, siginfo, context);
}
/*++
@@ -296,18 +342,7 @@ static void sigfpe_handler(int code, siginfo_t *siginfo, void *context)
}
}
- if (g_previous_sigfpe.sa_sigaction != NULL)
- {
- g_previous_sigfpe.sa_sigaction(code, siginfo, context);
- }
- else
- {
- // Restore the original or default handler and restart h/w exception
- restore_signal(code, &g_previous_sigfpe);
- }
-
- PROCNotifyProcessShutdown();
- PROCCreateCrashDumpIfEnabled();
+ invoke_previous_action(&g_previous_sigfpe, code, siginfo, context);
}
/*++
@@ -418,18 +453,7 @@ static void sigsegv_handler(int code, siginfo_t *siginfo, void *context)
}
}
- if (g_previous_sigsegv.sa_sigaction != NULL)
- {
- g_previous_sigsegv.sa_sigaction(code, siginfo, context);
- }
- else
- {
- // Restore the original or default handler and restart h/w exception
- restore_signal(code, &g_previous_sigsegv);
- }
-
- PROCNotifyProcessShutdown();
- PROCCreateCrashDumpIfEnabled();
+ invoke_previous_action(&g_previous_sigsegv, code, siginfo, context);
}
/*++
@@ -453,19 +477,8 @@ static void sigtrap_handler(int code, siginfo_t *siginfo, void *context)
}
}
- if (g_previous_sigtrap.sa_sigaction != NULL)
- {
- g_previous_sigtrap.sa_sigaction(code, siginfo, context);
- }
- else
- {
- // We abort instead of restore the original or default handler and returning
- // because returning from a SIGTRAP handler continues execution past the trap.
- PROCAbort();
- }
-
- PROCNotifyProcessShutdown();
- PROCCreateCrashDumpIfEnabled();
+ // The signal doesn't restart, returning from a SIGTRAP handler continues execution past the trap.
+ invoke_previous_action(&g_previous_sigtrap, code, siginfo, context, /* signalRestarts */ false);
}
/*++
@@ -492,18 +505,7 @@ static void sigbus_handler(int code, siginfo_t *siginfo, void *context)
}
}
- if (g_previous_sigbus.sa_sigaction != NULL)
- {
- g_previous_sigbus.sa_sigaction(code, siginfo, context);
- }
- else
- {
- // Restore the original or default handler and restart h/w exception
- restore_signal(code, &g_previous_sigbus);
- }
-
- PROCNotifyProcessShutdown();
- PROCCreateCrashDumpIfEnabled();
+ invoke_previous_action(&g_previous_sigbus, code, siginfo, context);
}
/*++
@@ -521,9 +523,7 @@ static void sigint_handler(int code, siginfo_t *siginfo, void *context)
{
PROCNotifyProcessShutdown();
- // Restore the original or default handler and resend signal
- restore_signal(code, &g_previous_sigint);
- kill(gPID, code);
+ restore_signal_and_resend(code, &g_previous_sigint);
}
/*++
@@ -541,9 +541,7 @@ static void sigquit_handler(int code, siginfo_t *siginfo, void *context)
{
PROCNotifyProcessShutdown();
- // Restore the original or default handler and resend signal
- restore_signal(code, &g_previous_sigquit);
- kill(gPID, code);
+ restore_signal_and_resend(code, &g_previous_sigquit);
}
#endif // !HAVE_MACH_EXCEPTIONS
@@ -569,10 +567,7 @@ static void sigterm_handler(int code, siginfo_t *siginfo, void *context)
}
else
{
- if (g_previous_sigterm.sa_sigaction != NULL)
- {
- g_previous_sigterm.sa_sigaction(code, siginfo, context);
- }
+ restore_signal_and_resend(SIGTERM, &g_previous_sigterm);
}
}
@@ -612,9 +607,23 @@ static void inject_activation_handler(int code, siginfo_t *siginfo, void *contex
CONTEXTToNativeContext(&winContext, ucontext);
}
}
- else if (g_previous_activation.sa_sigaction != NULL)
+ else
{
- g_previous_activation.sa_sigaction(code, siginfo, context);
+ // Call the original handler when it is not ignored or default (terminate).
+ if (g_previous_activation.sa_flags & SA_SIGINFO)
+ {
+ _ASSERTE(g_previous_activation.sa_sigaction != NULL);
+ g_previous_activation.sa_sigaction(code, siginfo, context);
+ }
+ else
+ {
+ if (g_previous_activation.sa_handler != SIG_IGN &&
+ g_previous_activation.sa_handler != SIG_DFL)
+ {
+ _ASSERTE(g_previous_activation.sa_handler != NULL);
+ g_previous_activation.sa_handler(code);
+ }
+ }
}
}
#endif
@@ -832,13 +841,9 @@ void handle_signal(int signal_id, SIGFUNC sigfunc, struct sigaction *previousAct
struct sigaction newAction;
newAction.sa_flags = SA_RESTART | additionalFlags;
-#if HAVE_SIGINFO_T
newAction.sa_handler = NULL;
newAction.sa_sigaction = sigfunc;
newAction.sa_flags |= SA_SIGINFO;
-#else /* HAVE_SIGINFO_T */
- newAction.sa_handler = SIG_DFL;
-#endif /* HAVE_SIGINFO_T */
sigemptyset(&newAction.sa_mask);
#ifdef INJECT_ACTIVATION_SIGNAL
@@ -891,3 +896,21 @@ void restore_signal(int signal_id, struct sigaction *previousAction)
errno, strerror(errno));
}
}
+
+/*++
+Function :
+ restore_signal_and_resend
+
+ restore handler for specified signal and signal the process
+
+Parameters :
+ int signal_id : signal to handle
+ previousAction : previous sigaction struct to restore
+
+ (no return value)
+--*/
+void restore_signal_and_resend(int signal_id, struct sigaction* previousAction)
+{
+ restore_signal(signal_id, previousAction);
+ kill(gPID, signal_id);
+} \ No newline at end of file
diff --git a/src/pal/src/synchmgr/synchmanager.cpp b/src/pal/src/synchmgr/synchmanager.cpp
index 349b3de135..1bdfb9426b 100644
--- a/src/pal/src/synchmgr/synchmanager.cpp
+++ b/src/pal/src/synchmgr/synchmanager.cpp
@@ -1657,7 +1657,11 @@ namespace CorUnix
// Call the termination request handler if one is registered.
if (g_terminationRequestHandler != NULL)
{
- g_terminationRequestHandler();
+ // The process will terminate normally by calling exit.
+ // We use an exit code of '128 + signo'. This is a convention used in popular
+ // shells to calculate an exit code when the process was terminated by a signal.
+ // This is also used by the Process.ExitCode implementation.
+ g_terminationRequestHandler(128 + SIGTERM);
}
return 0;
diff --git a/src/vm/exceptionhandling.cpp b/src/vm/exceptionhandling.cpp
index 869084cff9..8e6da16e8b 100644
--- a/src/vm/exceptionhandling.cpp
+++ b/src/vm/exceptionhandling.cpp
@@ -16,6 +16,7 @@
#include "perfcounters.h"
#include "eventtrace.h"
#include "virtualcallstub.h"
+#include "utilcode.h"
#if defined(_TARGET_X86_)
#define USE_CURRENT_CONTEXT_IN_FILTER
@@ -203,10 +204,23 @@ static inline void UpdatePerformanceMetrics(CrawlFrame *pcfThisFrame, BOOL bIsRe
ETW::ExceptionLog::ExceptionThrown(pcfThisFrame, bIsRethrownException, bIsNewException);
}
-void ShutdownEEAndExitProcess()
+#ifdef FEATURE_PAL
+static LONG volatile g_termination_triggered = 0;
+
+void HandleTerminationRequest(int terminationExitCode)
{
- ForceEEShutdown(SCA_ExitProcessWhenShutdownComplete);
+ // We set a non-zero exit code to indicate the process didn't terminate cleanly.
+ // This value can be changed by the user by setting Environment.ExitCode in the
+ // ProcessExit event. We only start termination on the first SIGTERM signal
+ // to ensure we don't overwrite an exit code already set in ProcessExit.
+ if (InterlockedCompareExchange(&g_termination_triggered, 1, 0) == 0)
+ {
+ SetLatchedExitCode(terminationExitCode);
+
+ ForceEEShutdown(SCA_ExitProcessWhenShutdownComplete);
+ }
}
+#endif
void InitializeExceptionHandling()
{
@@ -229,7 +243,7 @@ void InitializeExceptionHandling()
PAL_SetGetGcMarkerExceptionCode(GetGcMarkerExceptionCode);
// Register handler for termination requests (e.g. SIGTERM)
- PAL_SetTerminationRequestHandler(ShutdownEEAndExitProcess);
+ PAL_SetTerminationRequestHandler(HandleTerminationRequest);
#endif // FEATURE_PAL
}