[webkit-reviews] review granted: [Bug 239919] Add option to JSC shell to wait for a USR2 signal before exiting to aid in collection of vmmaps : [Attachment 458620] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has granted Justin Michaud
<justin_michaud at apple.com>'s request for review:
Bug 239919: Add option to JSC shell to wait for a USR2 signal before exiting to
aid in collection of vmmaps
https://bugs.webkit.org/show_bug.cgi?id=239919

Attachment 458620: Patch

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




--- Comment #3 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 458620
  --> https://bugs.webkit.org/attachment.cgi?id=458620
Patch

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

r=me with comments.

> Source/JavaScriptCore/jsc.cpp:3041
> +    Lock waitToExitLock;
> +    Condition waitToExitCondition;
> +    bool mustWaitToExit = true;
> +

You can use WTF::BinarySemaphore instead.

> Source/JavaScriptCore/jsc.cpp:3049
> +	       {
> +		   Locker locker { waitToExitLock };
> +		   mustWaitToExit = false;
> +		   waitToExitCondition.notifyAll();
> +	       }

Ditto.

> Source/JavaScriptCore/jsc.cpp:3078
> +	   {
> +	       Locker locker { waitToExitLock };
> +	       while (mustWaitToExit)
> +		   waitToExitCondition.wait(waitToExitLock);
> +	   }
> +

Ditto.

>> Source/WTF/wtf/threads/Signals.h:66
>> +	case Signal::Usr: return std::make_tuple(SIGUSR2, std::nullopt);
> 
> Either I fixed a happy little bug, or I am about to feel the wrath of some
deep horrible bug that this works around

Currently, Signal::Usr is not used. So yeah, it is a bug, but it will not
affect on the existing code :)