Author: mturk Date: Mon Sep 14 15:30:26 2009 New Revision: 814697 URL: http://svn.apache.org/viewvc?rev=814697&view=rev Log: Cleanup signal API
Modified: commons/sandbox/runtime/trunk/src/main/native/include/acr_signals.h commons/sandbox/runtime/trunk/src/main/native/os/unix/signals.c commons/sandbox/runtime/trunk/src/main/native/os/win32/signals.c commons/sandbox/runtime/trunk/src/main/native/test/testsuite.c Modified: commons/sandbox/runtime/trunk/src/main/native/include/acr_signals.h URL: http://svn.apache.org/viewvc/commons/sandbox/runtime/trunk/src/main/native/include/acr_signals.h?rev=814697&r1=814696&r2=814697&view=diff ============================================================================== --- commons/sandbox/runtime/trunk/src/main/native/include/acr_signals.h (original) +++ commons/sandbox/runtime/trunk/src/main/native/include/acr_signals.h Mon Sep 14 15:30:26 2009 @@ -50,7 +50,7 @@ * @param sig signal to send. * @param to pid of the process */ -ACR_DECLARE(int) ACR_SendSignal(const acr_pchar_t *salt, int signum, int to); +ACR_DECLARE(int) ACR_RaiseSignal(const acr_pchar_t *salt, int signum, int to); #ifdef __cplusplus } Modified: commons/sandbox/runtime/trunk/src/main/native/os/unix/signals.c URL: http://svn.apache.org/viewvc/commons/sandbox/runtime/trunk/src/main/native/os/unix/signals.c?rev=814697&r1=814696&r2=814697&view=diff ============================================================================== --- commons/sandbox/runtime/trunk/src/main/native/os/unix/signals.c (original) +++ commons/sandbox/runtime/trunk/src/main/native/os/unix/signals.c Mon Sep 14 15:30:26 2009 @@ -53,7 +53,15 @@ return "Unknown signal (number)"; } -ACR_DECLARE(int) ACR_SendSignal(const char *salt, int signum, int to) +ACR_DECLARE(int) ACR_RaiseSignal(const char *salt, int signum, int to) { - return ACR_ENOTIMPL; + int rc; + if (to < 0) + rc = raise(signum); + else + rc = kill(to, signum); + if (rc) + return ACR_GET_OS_ERROR() + else + return 0; } Modified: commons/sandbox/runtime/trunk/src/main/native/os/win32/signals.c URL: http://svn.apache.org/viewvc/commons/sandbox/runtime/trunk/src/main/native/os/win32/signals.c?rev=814697&r1=814696&r2=814697&view=diff ============================================================================== --- commons/sandbox/runtime/trunk/src/main/native/os/win32/signals.c (original) +++ commons/sandbox/runtime/trunk/src/main/native/os/win32/signals.c Mon Sep 14 15:30:26 2009 @@ -107,8 +107,8 @@ return memcmp(digest, msg->cookie, 20); } -/* Write an empty message to the pipe. - * So we can bail out from the ConnectNamedPipe call +/* Write an empty message to the pipe + * so we can bail out from the ConnectNamedPipe call */ static void ping_sig_pipe(int state) { @@ -116,8 +116,12 @@ acr_sig_msg_t msg; signal_handlers_running = state; - if (IS_INVALID_HANDLE(sig_pipe_handle)) + if (IS_INVALID_HANDLE(sig_pipe_handle)) { + /* Pipe is already closed. + * Nothing to do. + */ return; + } memset(&msg, 0, sizeof(acr_sig_msg_t)); CallNamedPipeW(sig_pipe_name, &msg, @@ -213,19 +217,9 @@ break; } if (posix_signal) { - /* Sync with callback handler */ + /* Sync with signal callback handler */ EnterCriticalSection(&signal_lock); -#if defined(_MSC_VER) && (_MSC_VER >= 1300) - /* XXX: Do we really need an atomic op here? - */ - _InterlockedOr(¤t_signal_queue, sigmask(posix_signal)); -#else - /* We don't have compiler intrinsic. - * CriticalSection should guard the value for the majority - * of cases. - */ current_signal_queue |= sigmask(posix_signal); -#endif /* Fire the signal events. * The first locked listener will handle * the signal. @@ -253,80 +247,72 @@ HANDLE pipe = (HANDLE)param; acr_sig_msg_t msg; + memset(&msg, 0, sizeof(acr_sig_msg_t)); + if (!ReadFile(pipe, + &msg, + (DWORD)sizeof(acr_sig_msg_t), + &read, + NULL)) { + /* Read failed. */ + goto cleanup; + } + if (!signal_handlers_running) { + /* We just received a message + * probably from ping_sig_pipe and the + * signal subsytem is marked for shutdown. + * Close the pipe and bail out. + */ + goto cleanup; + } + if (read != sizeof(acr_sig_msg_t)) { + /* Invalid message size. + */ #if defined(DEBUG) - fprintf(stdout, "Started read thread\n"); - fflush(stdout); -#endif - for (;;) { - memset(&msg, 0, sizeof(acr_sig_msg_t)); - if (!ReadFile(pipe, - &msg, - (DWORD)sizeof(acr_sig_msg_t), - &read, - NULL)) { - /* Read failed. */ - break; - } - if (read != sizeof(acr_sig_msg_t)) { - /* Invalid message size. - */ -#if defined(DEBUG) - fprintf(stderr, "Received invalid message size %d\n", read); - fflush(stderr); -#endif - break; - } - if (verify_security_cookie(&msg, sig_pipe_salt)) { - /* Invalid message signature. - */ -#if defined(DEBUG) - fprintf(stderr, "Received invalid message signature\n"); - fflush(stderr); + fprintf(stderr, "Received invalid message size %d\n", read); + fflush(stderr); #endif - break; - } + goto cleanup; + } + if (verify_security_cookie(&msg, sig_pipe_salt)) { + /* Invalid message signature. + */ #if defined(DEBUG) - fprintf(stdout, "Received valid signal %d from %d\n", - msg.signal, msg.sender); - fflush(stdout); + fprintf(stderr, "Received invalid message signature\n"); + fflush(stderr); #endif - msg.sender = GetCurrentProcessId(); - /* Write the message back with our pid as sender. - * We don't care about the result of the write operation. - */ - WriteFile(pipe, &msg, (DWORD)sizeof(acr_sig_msg_t), &read, NULL); - FlushFileBuffers(pipe); + goto cleanup; + } + msg.sender = GetCurrentProcessId(); + /* Write the message back with our pid as sender. + * We don't care about the result of the write operation. + */ + WriteFile(pipe, &msg, (DWORD)sizeof(acr_sig_msg_t), &read, NULL); + FlushFileBuffers(pipe); + DisconnectNamedPipe(pipe); + CloseHandle(pipe); + /* Sync with signal handler */ + EnterCriticalSection(&signal_lock); + current_signal_queue |= sigmask(msg.signal); + /* Notify the signal monitor thread. + * Signal monitor thread will interrupt all waiters + * and dispatch the signals if there are no waiters pending. + */ + SetEvent(dll_auto_hevent); + LeaveCriticalSection(&signal_lock); + + return 0; +cleanup: + if (IS_VALID_HANDLE(pipe)) { DisconnectNamedPipe(pipe); CloseHandle(pipe); - - /* Deliver a signal */ - /* Sync with callback handler */ - EnterCriticalSection(&signal_lock); -#if defined(_MSC_VER) && (_MSC_VER >= 1300) - /* XXX: Do we really need an atomic op here? - */ - _InterlockedOr(¤t_signal_queue, sigmask(msg.signal)); -#else - /* We don't have compiler intrinsic. - * CriticalSection should guard the value for the majority - * of cases. - */ - current_signal_queue |= sigmask(msg.signal); -#endif - /* Fire the signal events. - * The first locked listener will handle - * the signal. - */ - SetEvent(dll_auto_hevent); - LeaveCriticalSection(&signal_lock); } - CloseHandle(pipe); return 0; } static DWORD WINAPI main_signal_monitor(LPVOID unused) { DWORD rc; + while (signal_handlers_running) { rc = WaitForSingleObject(dll_auto_hevent, INFINITE); if (rc == WAIT_OBJECT_0) { @@ -336,15 +322,30 @@ if (ACR_SIGNAL_NWAITERS() == 0) { LeaveCriticalSection(&signal_lock); rc = ACR_DeliverSignals(); + if (rc == ACR_INCOMPLETE) { + /* Signal delivery requires the + * shutdown + */ + signal_handlers_running = 0; + break; + } } else { LeaveCriticalSection(&signal_lock); + /* Since we have waiters signal the + * main interrupt event. + * Each of the waiters must call + * ACR_DeliverSignals(). + * The last one will reset the event + * and the first one will handle the + * signal queue. + */ SetEvent(dll_psig_handle); } } else { - /* Anything else is a fault. - * Terminate the monitor thread + /* Anything else means we have to exit. + * Terminate the monitor thread. */ signal_handlers_running = 0; break; @@ -352,11 +353,12 @@ } return 0; } -/* Main signal thread is responsible for handling the signal - * events. + +/* Main signal thread is responsible for handling the + * listening of external signal messages. * Once created it will remain running until application exit, * so there is no need for any thread cleanup since we cannot - * leak single thread. + * leak a single thread. */ static DWORD WINAPI main_signal_thread(LPVOID unused) { @@ -374,7 +376,7 @@ sig_pipe_instances, (DWORD)sizeof(acr_sig_msg_t), (DWORD)sizeof(acr_sig_msg_t), - 2000, + 1000, ACR_GetSaWithNullDacl()); if (IS_INVALID_HANDLE(sig_pipe_handle)) { /* Failed to create signal messaging pipe @@ -405,14 +407,14 @@ #endif if (connected && signal_handlers_running) { /* We have connected client. - * Start the cleint worker thread + * Start the client worker thread */ HANDLE wt; DWORD wi; if (!(wt = CreateThread(NULL, 0, read_signal_thread, - (LPVOID)sig_pipe_handle, + sig_pipe_handle, 0, &wi))) { @@ -425,6 +427,8 @@ /* Connect failed. * Close the pipe and try again. */ + if (connected) + DisconnectNamedPipe(sig_pipe_handle); CloseHandle(sig_pipe_handle); } sig_pipe_handle = INVALID_HANDLE_VALUE; @@ -471,20 +475,23 @@ signal_handlers[SIGBUS] = default_signal_handler; signal_handlers[SIGSEGV] = default_signal_handler; signal_handlers[SIGKILL] = default_signal_handler; - signal_handlers[SIGTERM] = default_signal_handler; + /* Default handlers for console events. + */ signal_handlers[SIGINT] = SIG_DFL; + signal_handlers[SIGTERM] = SIG_DFL; + signal_handlers[SIGHUP] = SIG_DFL; /* Get the global signal pipe name. * Combined from pid and ACR_NUMSIG. */ pipe_name_from_pid(sig_pipe_name, GetCurrentProcessId(), ACR_NUMSIG); sig_pipe_handle = CreateNamedPipeW(sig_pipe_name, - PIPE_ACCESS_DUPLEX, /* ###: We only read */ + PIPE_ACCESS_DUPLEX, PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT, sig_pipe_instances, (DWORD)sizeof(acr_sig_msg_t), (DWORD)sizeof(acr_sig_msg_t), - 2000, + 1000, ACR_GetSaWithNullDacl()); if (IS_INVALID_HANDLE(sig_pipe_handle)) { /* Failed to create signal messaging pipe @@ -572,17 +579,10 @@ } /* Remove the signal from the queue */ -#if defined(_MSC_VER) && (_MSC_VER >= 1300) - /* XXX: Do we really need an atomic op here? - */ - _InterlockedAnd(¤t_signal_queue, ~sigmask(msg.signal)); -#else current_signal_queue &= ~sigmask(i); -#endif if (sig != SIG_IGN && sig != SIG_ERR) { LeaveCriticalSection(&signal_lock); - /* TODO: Handle default signals */ - sig(i); + (*sig)(i); EnterCriticalSection(&signal_lock); break; } @@ -605,25 +605,45 @@ return "Unknown signal (number)"; } -ACR_DECLARE(int) ACR_SendSignal(const wchar_t *salt, int signum, int to) +ACR_DECLARE(int) ACR_RaiseSignal(const wchar_t *salt, int signum, int to) { - DWORD read; + DWORD read = 0; acr_sig_msg_t msg; - acr_sig_msg_t buf; + acr_sig_msg_t res; wchar_t name[64]; + if (signum < 1 || signum >= ACR_NUMSIG) + return ACR_EINVAL; + if (to < 0 || to == (int)GetCurrentProcessId()) { + /* Standard raise() call + */ + EnterCriticalSection(&signal_lock); + current_signal_queue |= sigmask(signum); + /* Wake up the monitor thread. + */ + LeaveCriticalSection(&signal_lock); + SetEvent(dll_auto_hevent); + + return 0; + } pipe_name_from_pid(name, to, ACR_NUMSIG); make_security_cookie(&msg, salt, signum, to); + /* Send the message to the target process pipe. + */ if (CallNamedPipeW(name, &msg, (DWORD)sizeof(acr_sig_msg_t), - &buf, + &res, (DWORD)sizeof(acr_sig_msg_t), &read, NMPWAIT_USE_DEFAULT_WAIT)) { - return 0; + if (read == (DWORD)sizeof(acr_sig_msg_t) && + res.sender == (acr_uint32_t)to) + return 0; + else + return ACR_EACCES; } else { - ACR_GET_OS_ERROR(); + return ACR_GET_OS_ERROR(); } } Modified: commons/sandbox/runtime/trunk/src/main/native/test/testsuite.c URL: http://svn.apache.org/viewvc/commons/sandbox/runtime/trunk/src/main/native/test/testsuite.c?rev=814697&r1=814696&r2=814697&view=diff ============================================================================== --- commons/sandbox/runtime/trunk/src/main/native/test/testsuite.c (original) +++ commons/sandbox/runtime/trunk/src/main/native/test/testsuite.c Mon Sep 14 15:30:26 2009 @@ -538,10 +538,16 @@ #endif } else { + int rc; ppid = atoi(argv[0]); if (ppid == 0) return ACR_EINVAL; - ACR_SendSignal(NULL, SIGBUS, ppid); + rc = ACR_RaiseSignal(NULL, SIGBUS, ppid); + if (rc) { + char buf[256]; + fprintf(stderr, ACR_GetErrorString(rc, buf, sizeof(buf))); + fputc('\n', stderr); + } } return 0; }