Use dispatch queues for mach exceptions
authorkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 31 May 2017 22:45:07 +0000 (22:45 +0000)
committerkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 31 May 2017 22:45:07 +0000 (22:45 +0000)
https://bugs.webkit.org/show_bug.cgi?id=172775

Reviewed by Geoffrey Garen.

This patch adds support for using a dispatch queue to handle our
mach exceptions. We use a high priority concurrent dispatch queue
to handle our mach exceptions. We don't know the priority of the
thread whose exception we are handling so the most conservative
answer is to respond with a high priority. These events are both
rare and usually quite fast so it is likely not a significant cost
when the thread with an exception has a low priority.

* wtf/threads/Signals.cpp:
(WTF::startMachExceptionHandlerThread):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@217631 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WTF/ChangeLog
Source/WTF/wtf/threads/Signals.cpp

index 5712dc0..7abe64d 100644 (file)
@@ -1,5 +1,23 @@
 2017-05-31  Keith Miller  <keith_miller@apple.com>
 
+        Use dispatch queues for mach exceptions
+        https://bugs.webkit.org/show_bug.cgi?id=172775
+
+        Reviewed by Geoffrey Garen.
+
+        This patch adds support for using a dispatch queue to handle our
+        mach exceptions. We use a high priority concurrent dispatch queue
+        to handle our mach exceptions. We don't know the priority of the
+        thread whose exception we are handling so the most conservative
+        answer is to respond with a high priority. These events are both
+        rare and usually quite fast so it is likely not a significant cost
+        when the thread with an exception has a low priority.
+
+        * wtf/threads/Signals.cpp:
+        (WTF::startMachExceptionHandlerThread):
+
+2017-05-31  Keith Miller  <keith_miller@apple.com>
+
         Reland r216808, underlying lldb bug has been fixed.
         https://bugs.webkit.org/show_bug.cgi?id=172759
 
index 707ca9c..7f24706 100644 (file)
@@ -39,6 +39,7 @@ extern "C" {
 #include <signal.h>
 
 #if HAVE(MACH_EXCEPTIONS)
+#include <dispatch/dispatch.h>
 #include <mach/mach.h>
 #include <mach/thread_act.h>
 #endif
@@ -78,31 +79,38 @@ static void startMachExceptionHandlerThread()
         if (mach_port_insert_right(mach_task_self(), exceptionPort, exceptionPort, MACH_MSG_TYPE_MAKE_SEND) != KERN_SUCCESS)
             CRASH();
 
-        // FIXME: This should use a dispatch queue.
-        // See: https://bugs.webkit.org/show_bug.cgi?id=172003
-        (void)Thread::create(
-            "WTF Mach Exception Thread", [] () {
-                union Message {
-                    mach_msg_header_t header;
-                    char data[maxMessageSize];
-                };
-                Message messageHeaderIn;
-                Message messageHeaderOut;
-
-                while (true) {
-                    kern_return_t messageResult = mach_msg(&messageHeaderIn.header, MACH_RCV_MSG, 0, maxMessageSize, exceptionPort, MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL);
-                    if (messageResult == KERN_SUCCESS) {
-                        if (!mach_exc_server(&messageHeaderIn.header, &messageHeaderOut.header))
-                            CRASH();
-
-                        messageResult = mach_msg(&messageHeaderOut.header, MACH_SEND_MSG, messageHeaderOut.header.msgh_size, 0, messageHeaderOut.header.msgh_local_port, 0, MACH_PORT_NULL);
-                        RELEASE_ASSERT(messageResult == KERN_SUCCESS);
-                    } else {
-                        dataLogLn("Failed to receive mach message due to ", mach_error_string(messageResult));
-                        RELEASE_ASSERT_NOT_REACHED();
-                    }
-                }
-        }).leakRef();
+        // It's not clear that this needs to be the high priority queue but it should be rare and it might be
+        // handling exceptions from high priority threads. Anyway, our handlers should be very fast anyway so it's
+        // probably not the end of the world if we handle a low priority exception on a high priority queue.
+        dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0);
+        dispatch_source_t source = dispatch_source_create(DISPATCH_SOURCE_TYPE_MACH_RECV, exceptionPort, 0, queue);
+        RELEASE_ASSERT_WITH_MESSAGE(source, "We need to ensure our source was created.");
+
+        // We should never cancel our handler since it's a permanent thing so we don't add a cancel handler.
+        dispatch_source_set_event_handler(source, ^{
+            // the leaks tool will get mad at us if we don't pretend to watch the source.
+            UNUSED_PARAM(source);
+            union Message {
+                mach_msg_header_t header;
+                char data[maxMessageSize];
+            };
+            Message messageHeaderIn;
+            Message messageHeaderOut;
+
+            kern_return_t messageResult = mach_msg(&messageHeaderIn.header, MACH_RCV_MSG, 0, maxMessageSize, exceptionPort, MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL);
+            if (messageResult == KERN_SUCCESS) {
+                if (!mach_exc_server(&messageHeaderIn.header, &messageHeaderOut.header))
+                    CRASH();
+
+                messageResult = mach_msg(&messageHeaderOut.header, MACH_SEND_MSG, messageHeaderOut.header.msgh_size, 0, messageHeaderOut.header.msgh_local_port, 0, MACH_PORT_NULL);
+                RELEASE_ASSERT(messageResult == KERN_SUCCESS);
+            } else {
+                dataLogLn("Failed to receive mach message due to ", mach_error_string(messageResult));
+                RELEASE_ASSERT_NOT_REACHED();
+            }
+        });
+
+        dispatch_resume(source);
     });
 }