You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by mt...@apache.org on 2009/09/14 17:30:26 UTC

svn commit: r814697 - in /commons/sandbox/runtime/trunk/src/main/native: include/acr_signals.h os/unix/signals.c os/win32/signals.c test/testsuite.c

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(&current_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(&current_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(&current_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;
 }