diff --git a/mcs/class/Mono.Posix/Mono.Unix/UnixSignal.cs b/mcs/class/Mono.Posix/Mono.Unix/UnixSignal.cs index b71743f9102a1..6c441e528ebb1 100644 --- a/mcs/class/Mono.Posix/Mono.Unix/UnixSignal.cs +++ b/mcs/class/Mono.Posix/Mono.Unix/UnixSignal.cs @@ -145,6 +145,8 @@ public unsafe bool Reset () set {Interlocked.Exchange (ref Info->count, value);} } + // signum, count, write_fd, pipecnt, and pipelock are read from a signal handler thread + // count and pipelock are both read and written from the signal handler thread [Map] struct SignalInfo { public int signum, count, read_fd, write_fd, pipecnt, pipelock, have_handler; diff --git a/mcs/class/Mono.Posix/Test/Mono.Unix/UnixSignalTest.cs b/mcs/class/Mono.Posix/Test/Mono.Unix/UnixSignalTest.cs index 14613104d9b7c..d621ad51d4533 100644 --- a/mcs/class/Mono.Posix/Test/Mono.Unix/UnixSignalTest.cs +++ b/mcs/class/Mono.Posix/Test/Mono.Unix/UnixSignalTest.cs @@ -456,6 +456,7 @@ static void CloseSignals (UnixSignal[] signals) s.Close (); } + // Create thread that issues many signals from a set of harmless signals static Thread CreateRaiseStormThread (int max) { return new Thread (delegate () { @@ -486,6 +487,7 @@ public void TestAddRemove () CloseSignals (usignals); } + // Create thread that repeatedly registers then unregisters signal handlers static Thread CreateSignalCreatorThread () { return new Thread (delegate () { @@ -523,6 +525,7 @@ public void TestWaitAny () CloseSignals (usignals); } + // Create thread that blocks until at least one of the given signals is received static Thread CreateWaitAnyThread (params UnixSignal[] usignals) { return new Thread (delegate () { diff --git a/support/signal.c b/support/signal.c index c945729c6a12c..993e2e216618b 100644 --- a/support/signal.c +++ b/support/signal.c @@ -107,6 +107,10 @@ int Mono_Posix_FromRealTimeSignum (int offset, int *r) #ifndef HOST_WIN32 +// Atomicity rules: Fields of signal_info read or written by the signal handler +// (see UnixSignal.cs) should be read and written using atomic functions. +// (For simplicity, we're protecting some things we don't strictly need to.) + // Because we are in MonoPosixHelper, we are banned from linking mono. // We can still use atomic.h because that's all inline functions-- // unless WAPI_NO_ATOMIC_ASM is defined, in which case atomic.h calls linked functions. @@ -170,9 +174,9 @@ keep_trying (int r) // This tiny ad-hoc read/write lock is needed because of the very specific // synchronization needed between default_handler and teardown_pipes: // - Many default_handlers can be running at once -// - External forces already ensure only one teardown_pipes runs at once -// - If a teardown_pipes interrupts a default_handler, it must block -// - If a default_handler interrupts a teardown_pipes, it must *not* block +// - The signals_mutex already ensures only one teardown_pipes runs at once +// - If teardown_pipes starts while a default_handler is ongoing, it must block +// - If default_handler starts while a teardown_pipes is ongoing, it must *not* block // Locks are implemented as ints. // The lock is split into a teardown bit and a handler count (sign bit unused). @@ -196,7 +200,7 @@ acquire_pipelock_teardown (int *lock) } // Now wait for all handlers to complete. while (1) { - if (0 == PIPELOCK_GET_COUNT(lockvalue_draining)) + if (0 == PIPELOCK_GET_COUNT (lockvalue_draining)) break; // We now hold the lock. // Handler is still running, spin until it completes. sched_yield (); // We can call this because !defined(HOST_WIN32) @@ -267,10 +271,10 @@ default_handler (int signum) continue; // Teardown is occurring on this object, no one to send to. fd = mph_int_get (&h->write_fd); - if (fd > 0) { + if (fd > 0) { // If any listener exists to write to int j,pipecounter; - char c = signum; - pipecounter = mph_int_get (&h->pipecnt); + char c = signum; // (Value is meaningless) + pipecounter = mph_int_get (&h->pipecnt); // Write one byte per pipe listener for (j = 0; j < pipecounter; ++j) { int r; do { r = write (fd, &c, 1); } while (keep_trying (r)); @@ -282,13 +286,14 @@ default_handler (int signum) static pthread_mutex_t signals_mutex = PTHREAD_MUTEX_INITIALIZER; +// A UnixSignal object is being constructed void* Mono_Unix_UnixSignal_install (int sig) { #if defined(HAVE_SIGNAL) int i; - signal_info* h = NULL; - int have_handler = 0; + signal_info* h = NULL; // signals[] slot to install to + int have_handler = 0; // Candidates for signal_info handler fields void* handler = NULL; if (acquire_mutex (&signals_mutex) == -1) @@ -302,13 +307,16 @@ Mono_Unix_UnixSignal_install (int sig) if (sinfo.sa_handler != SIG_DFL || (void*)sinfo.sa_sigaction != (void*)SIG_DFL) { pthread_mutex_unlock (&signals_mutex); errno = EADDRINUSE; - return NULL; + return NULL; // This is an rt signal with an existing handler. Bail out. } } #endif /*defined (SIGRTMIN) && defined (SIGRTMAX)*/ + // Scan through signals list looking for (1) an unused spot (2) a usable value for handler for (i = 0; i < NUM_SIGNALS; ++i) { - if (h == NULL && signals [i].signum == 0) { + int just_installed = 0; + // We're still looking for a signal_info spot, and this one is available: + if (h == NULL && mph_int_get (&signals [i].signum) == 0) { h = &signals [i]; h->handler = signal (sig, default_handler); if (h->handler == SIG_ERR) { @@ -317,27 +325,32 @@ Mono_Unix_UnixSignal_install (int sig) break; } else { - h->have_handler = 1; + just_installed = 1; } } - if (!have_handler && signals [i].signum == sig && + // Check if this slot has a "usable" (not installed by this file) handler-to-restore-later: + // (On the first signal to be installed, signals [i] will be == h when this happens.) + if (!have_handler && (just_installed || mph_int_get (&signals [i].signum) == sig) && signals [i].handler != default_handler) { have_handler = 1; handler = signals [i].handler; } - if (h && have_handler) + if (h && have_handler) // We have everything we need break; } - if (h && have_handler) { + if (h) { + // If we reached here without have_handler, this means that default_handler + // was set as the signal handler before the first UnixSignal object was installed. + g_assert (have_handler); + + // Overwrite the tenative handler we set a moment ago with a known-usable one + h->handler = handler; h->have_handler = 1; - h->handler = handler; - } - if (h) { mph_int_set (&h->count, 0); - mph_int_set (&h->signum, sig); mph_int_set (&h->pipecnt, 0); + mph_int_set (&h->signum, sig); } release_mutex (&signals_mutex); @@ -355,16 +368,17 @@ count_handlers (int signum) int i; int count = 0; for (i = 0; i < NUM_SIGNALS; ++i) { - if (signals [i].signum == signum) + if (mph_int_get (&signals [i].signum) == signum) ++count; } return count; } +// A UnixSignal object is being Disposed int Mono_Unix_UnixSignal_uninstall (void* info) { -#if defined(HAVE_SIGNAL) +#if defined(HAVE_SIGNAL) signal_info* h; int r = -1; @@ -377,14 +391,15 @@ Mono_Unix_UnixSignal_uninstall (void* info) errno = EINVAL; else { /* last UnixSignal -- we can unregister */ - if (h->have_handler && count_handlers (h->signum) == 1) { - mph_sighandler_t p = signal (h->signum, h->handler); + int signum = mph_int_get (&h->signum); + if (h->have_handler && count_handlers (signum) == 1) { + mph_sighandler_t p = signal (signum, h->handler); if (p != SIG_ERR) r = 0; h->handler = NULL; h->have_handler = 0; } - h->signum = 0; + mph_int_set (&h->signum, 0); } release_mutex (&signals_mutex); @@ -396,6 +411,7 @@ Mono_Unix_UnixSignal_uninstall (void* info) #endif } +// Set up a signal_info to begin waiting for signal static int setup_pipes (signal_info** signals, int count, struct pollfd *fd_structs, int *currfd) { @@ -407,21 +423,22 @@ setup_pipes (signal_info** signals, int count, struct pollfd *fd_structs, int *c h = signals [i]; - if (mph_int_get (&h->pipecnt) == 0) { + if (mph_int_get (&h->pipecnt) == 0) { // First listener for this signal_info if ((r = pipe (filedes)) != 0) { break; } - h->read_fd = filedes [0]; - h->write_fd = filedes [1]; + mph_int_set (&h->read_fd, filedes [0]); + mph_int_set (&h->write_fd, filedes [1]); } mph_int_inc (&h->pipecnt); - fd_structs[*currfd].fd = h->read_fd; + fd_structs[*currfd].fd = mph_int_get (&h->read_fd); fd_structs[*currfd].events = POLLIN; - ++(*currfd); + ++(*currfd); // count is verified less than NUM_SIGNALS by caller } return r; } +// Cleanup a signal_info after waiting for signal static void teardown_pipes (signal_info** signals, int count) { @@ -429,23 +446,28 @@ teardown_pipes (signal_info** signals, int count) for (i = 0; i < count; ++i) { signal_info* h = signals [i]; - if (mph_int_dec_test (&h->pipecnt)) { + if (mph_int_dec_test (&h->pipecnt)) { // Final listener for this signal_info acquire_pipelock_teardown (&h->pipelock); - if (h->read_fd != 0) - close (h->read_fd); - if (h->write_fd != 0) - close (h->write_fd); - h->read_fd = 0; - h->write_fd = 0; + int read_fd = mph_int_get (&h->read_fd); + int write_fd = mph_int_get (&h->write_fd); + if (read_fd != 0) + close (read_fd); + if (write_fd != 0) + close (write_fd); + mph_int_set (&h->read_fd, 0); + mph_int_set (&h->write_fd, 0); release_pipelock_teardown (&h->pipelock); } } } +// Given pipes set up, wait for a byte to arrive on one of them static int wait_for_any (signal_info** signals, int count, int *currfd, struct pollfd* fd_structs, int timeout, Mono_Posix_RuntimeIsShuttingDown shutting_down) { int r, idx; + // Poll until one of this signal_info's pipes is ready to read. + // Once a second, stop to check if the VM is shutting down. do { struct timeval tv; struct timeval *ptv = NULL; @@ -460,7 +482,7 @@ wait_for_any (signal_info** signals, int count, int *currfd, struct pollfd* fd_s idx = -1; if (r == 0) idx = timeout; - else if (r > 0) { + else if (r > 0) { // The pipe[s] are ready to read. int i; for (i = 0; i < count; ++i) { signal_info* h = signals [i]; @@ -468,7 +490,7 @@ wait_for_any (signal_info** signals, int count, int *currfd, struct pollfd* fd_s int r; char c; do { - r = read (h->read_fd, &c, 1); + r = read (mph_int_get (&h->read_fd), &c, 1); } while (keep_trying (r) && !shutting_down ()); if (idx == -1) idx = i;