Attachment 340279 Details for Bug 185595 – Patch

[patch] Patch

bug-185595-20180513165517.patch (text/plain), 4.27 KB, created by Geoffrey Garen on 2018-05-13 16:55:17 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Geoffrey Garen

Created: 2018-05-13 16:55:17 PDT

Size: 4.27 KB

patch

obsolete

>Index: Source/WTF/ChangeLog
>===================================================================
>--- Source/WTF/ChangeLog	(revision 231745)
>+++ Source/WTF/ChangeLog	(working copy)
>@@ -1,3 +1,20 @@
>+2018-05-13  Geoffrey Garen  <ggaren@apple.com>
>+
>+        Simplified Mach exception handling
>+        https://bugs.webkit.org/show_bug.cgi?id=185595
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * wtf/threads/Signals.cpp:
>+        (WTF::startMachExceptionHandlerThread): Use mach_msg_server_once instead
>+        of duplicating its functionality. Separate error handling logic from
>+        program logic to help program logic stand out. Use
>+        DISPATCH_TARGET_QUEUE_* instead of explicitly fetching a queue.
>+
>+        Also, we don't need the high priority queue. The kernel donates a
>+        priority voucher from the exception thread to the receiver thread, and
>+        mach_msg_server_once takes care to forward that voucher.
>+
> 2018-05-13  Filip Pizlo  <fpizlo@apple.com>
> 
>         Disable pointer poisoning
>Index: Source/WTF/wtf/threads/Signals.cpp
>===================================================================
>--- Source/WTF/wtf/threads/Signals.cpp	(revision 231743)
>+++ Source/WTF/wtf/threads/Signals.cpp	(working copy)
>@@ -73,41 +73,21 @@ static void startMachExceptionHandlerThr
> {
>     static std::once_flag once;
>     std::call_once(once, [] {
>-        if (mach_port_allocate(mach_task_self(), MACH_PORT_RIGHT_RECEIVE, &exceptionPort) != KERN_SUCCESS)
>-            CRASH();
>+        kern_return_t kr = mach_port_allocate(mach_task_self(), MACH_PORT_RIGHT_RECEIVE, &exceptionPort);
>+        RELEASE_ASSERT(kr == KERN_SUCCESS);
>+        kr = mach_port_insert_right(mach_task_self(), exceptionPort, exceptionPort, MACH_MSG_TYPE_MAKE_SEND);
>+        RELEASE_ASSERT(kr == KERN_SUCCESS);
>+
>+        dispatch_source_t source = dispatch_source_create(
>+            DISPATCH_SOURCE_TYPE_MACH_RECV, exceptionPort, 0, DISPATCH_TARGET_QUEUE_DEFAULT);
>+        RELEASE_ASSERT(source);
> 
>-        if (mach_port_insert_right(mach_task_self(), exceptionPort, exceptionPort, MACH_MSG_TYPE_MAKE_SEND) != KERN_SUCCESS)
>-            CRASH();
>-
>-        // 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();
>-            }
>+            UNUSED_PARAM(source); // Silence the leaks tool.
>+
>+            kern_return_t kr = mach_msg_server_once(
>+                mach_exc_server, maxMessageSize, exceptionPort, MACH_MSG_TIMEOUT_NONE);
>+            RELEASE_ASSERT(kr == KERN_SUCCESS);
>         });
> 
>         dispatch_resume(source);

Flags:

keith_miller: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 185595: 340279