[webkit-reviews] review granted: [Bug 172775] Use dispatch queues for mach exceptions : [Attachment 311634] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Keith Miller
<keith_miller at apple.com>'s request for review:
Bug 172775: Use dispatch queues for mach exceptions
https://bugs.webkit.org/show_bug.cgi?id=172775

Attachment 311634: Patch

https://bugs.webkit.org/attachment.cgi?id=311634&action=review




--- Comment #6 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 311634
  --> https://bugs.webkit.org/attachment.cgi?id=311634
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=311634&action=review

r=me

> Source/WTF/wtf/threads/Signals.cpp:87
> +	   RELEASE_ASSERT_WITH_MESSAGE(source, "We need to ensure our source
was created");

dispatch_source_create doesn't use null as an error condition return value. So
you should remove this assert.

> Source/WTF/wtf/threads/Signals.cpp:89
> +	   dispatch_source_set_event_handler(source, ^{

You need to keep a pointer to your dispatch_source_t or it will appear to the
leaks tool that it has leaked. I'd suggest capturing a pointer in your block:

static dispatch_source_t s_source = source;

It might also work just to do this:

UNUSED_PARAM(source);

> Source/WTF/wtf/threads/Signals.cpp:113
> +	   dispatch_source_set_cancel_handler(source, ^{
> +	       // We should never cancel our handler since it's a permanent
thing.
> +	       RELEASE_ASSERT_NOT_REACHED();
> +	   });

No need for this.