Modified: trunk/Source/WTF/ChangeLog (217630 => 217631)
--- trunk/Source/WTF/ChangeLog 2017-05-31 22:30:13 UTC (rev 217630)
+++ trunk/Source/WTF/ChangeLog 2017-05-31 22:45:07 UTC (rev 217631)
@@ -1,5 +1,23 @@
2017-05-31 Keith Miller <[email protected]>
+ 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 <[email protected]>
+
Reland r216808, underlying lldb bug has been fixed.
https://bugs.webkit.org/show_bug.cgi?id=172759
Modified: trunk/Source/WTF/wtf/threads/Signals.cpp (217630 => 217631)
--- trunk/Source/WTF/wtf/threads/Signals.cpp 2017-05-31 22:30:13 UTC (rev 217630)
+++ trunk/Source/WTF/wtf/threads/Signals.cpp 2017-05-31 22:45:07 UTC (rev 217631)
@@ -39,6 +39,7 @@
#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 @@
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;
+ // 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.");
- 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();
+ // 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;
- 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();
+ 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);
});
}
