[webkit-reviews] review denied: [Bug 129265] Need to initialize VM stack data even when the VM is on an exclusive thread : [Attachment 225100] the patch.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Feb 24 16:03:24 PST 2014
Geoffrey Garen <ggaren at apple.com> has denied Mark Lam <mark.lam at apple.com>'s
request for review:
Bug 129265: Need to initialize VM stack data even when the VM is on an
exclusive thread
https://bugs.webkit.org/show_bug.cgi?id=129265
Attachment 225100: the patch.
https://bugs.webkit.org/attachment.cgi?id=225100&action=review
------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=225100&action=review
> Source/JavaScriptCore/runtime/JSLock.cpp:79
> void JSLockHolder::init()
> {
> - if (m_vm)
> + if (!m_vm->exclusiveThread)
> m_vm->apiLock().lock();
> +
> + m_didInitializeStackPointerAtVMEntry =
m_vm->initializeStackDataIfNeeded();
> }
JSLockHolder is a convenience object, but this patch would turn it into an API
that's required for correctness. I don't think that's a great match because we
already have a corresponding API that's required for correctness: JSLock.
Instead of copying JSLock logic up into JSLockHolder, let's move the exclusive
thread optimization down into JSLock.
> Source/WebCore/bindings/js/JSDOMBinding.cpp:157
> void reportException(ExecState* exec, JSValue exception, CachedScript*
cachedScript)
> {
> + APIEntryShim entryShim(exec);
Let's make this an ASSERT, and put the shim higher in the stack. Since this
function takes JSC:: arguments, it's likely incorrect for its caller not to
lock.
More information about the webkit-reviews
mailing list