[webkit-reviews] review denied: [Bug 128766] Web Inspector: CRASH when evaluating in console of JSContext RWI with disabled breakpoints : [Attachment 224988] the patch.

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has denied Mark Lam <mark.lam at apple.com>'s
request for review:
Bug 128766: Web Inspector: CRASH when evaluating in console of JSContext RWI
with disabled breakpoints
https://bugs.webkit.org/show_bug.cgi?id=128766

Attachment 224988: the patch.
https://bugs.webkit.org/attachment.cgi?id=224988&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=224988&action=review


> Source/JavaScriptCore/ChangeLog:8
> +	   Now that we allow more than one thread to drop VM locks, it is no
longer

We always allowed this, so you need to update the premise in your description.

> Source/JavaScriptCore/runtime/VM.cpp:894
> +VMEntryScope* VM::entryScope() const
> +{
> +    WTFThreadData& threadData = wtfThreadData();
> +    return reinterpret_cast<VMEntryScope*>(threadData.vmEntryScope());
> +}

It doesn't make sense to turn entryScope into a per-thread concept, for two
reasons:

(1) A thread can use more than one VM, and the VMs will stomp each others'
per-thread entry scope values.

(2) A null entryScope is used as a promise that nobody is using the VM right
now, which allows us to do things like destructively discard code. That signal
is only valid because entryScope is a global per-VM concept.