Bug 158446

<rdar://problem/26638397>

For the window, it works fine: thisValue: Cell: 0x1195fbfd0 (0x119560280:[JSDOMWindowShell, {}, NonArray, Proto:0x1195dbaf0, Leaf]), ID: 748 thisValue (after toThis): Cell: 0x1195fbfd0 (0x119560280:[JSDOMWindowShell, {}, NonArray, Proto:0x1195dbaf0, Leaf]), ID: 748

JSGlobalObject implements toThis() like so: JSValue JSGlobalObject::toThis(JSCell*, ExecState* exec, ECMAMode ecmaMode) { if (ecmaMode == StrictMode) return jsUndefined(); return exec->globalThisValue(); } Since objectProtoFuncToString() calls toThis() with StrictMode, this is why we end up with undefined. In the case of the Window, it works because thisValue is a JSDOMWindowShell, not a JSDOMWindow. Therefore, it does not subclass JSGlobalObject. I am not sure yet how to fix this though.

Note that even though I focused on Stringification of DedicatedWorkerGlobalScope. This toThis() bug is breaking more worker related things. The reason most of the W3C tests are failing is because self.hasOwnProperty() is broken in workers (because of the same toThis() bug), not because of stringification.

Geoff suggested we expose a Proxy to the WorkerGlobalScope to JS, instead of the WorkerGlobalScope itself. This is similar to what we do with Window / WindowShell already. However, my attempts to do this have not worked so far (weird crashes during GC).

(In reply to comment #5) > Geoff suggested we expose a Proxy to the WorkerGlobalScope to JS, instead of > the WorkerGlobalScope itself. This is similar to what we do with Window / > WindowShell already. However, my attempts to do this have not worked so far > (weird crashes during GC). Can you upload your current working patch so that we can help see if there's something amiss?

Specifically, I think we want a JSProxy whose proxy type is PureForwardingProxyType and whose target is the WorkerGlobalScope.

Created attachment 280717 [details] Initial attempt (not working)

Ok, I attached my very initial attempt patch from yesterday, which is likely very wrong as it crashes badly.

Crash: Crashed Thread: 15 WebCore: Worker Exception Type: EXC_BAD_ACCESS (SIGSEGV) Exception Codes: KERN_INVALID_ADDRESS at 0x00000000000000c0 Exception Note: EXC_CORPSE_NOTIFY VM Regions Near 0xc0: --> __TEXT 0000000101aa7000-0000000101aa9000 [ 8K] r-x/rwx SM=COW /Volumes/VOLUME/*/WebKit.framework/Versions/A/XPCServices/com.apple.WebKit.WebContent.Development.xpc/Contents/MacOS/com.apple.WebKit.WebContent.Development Thread 15 Crashed:: WebCore: Worker 0 com.apple.JavaScriptCore 0x00000001031cb09f JSC::SlotVisitor::setMarkedAndAppendToMarkStack(JSC::JSCell*) + 95 (memory:2894) 1 com.apple.JavaScriptCore 0x0000000102fe6182 JSC::JSSymbolTableObject::visitChildren(JSC::JSCell*, JSC::SlotVisitor&) + 18 (WriteBarrier.h:92) 2 com.apple.JavaScriptCore 0x0000000102fda005 JSC::JSSegmentedVariableObject::visitChildren(JSC::JSCell*, JSC::SlotVisitor&) + 21 (SegmentedVector.h:106) 3 com.apple.JavaScriptCore 0x0000000102f79cfc JSC::JSGlobalObject::visitChildren(JSC::JSCell*, JSC::SlotVisitor&) + 28 (WriteBarrier.h:92) 4 com.apple.WebCore 0x0000000103fea829 WebCore::JSDOMGlobalObject::visitChildren(JSC::JSCell*, JSC::SlotVisitor&) + 25 (HashTraits.h:182) 5 com.apple.JavaScriptCore 0x00000001031cb35c JSC::SlotVisitor::drain() + 284 (SlotVisitor.cpp:226) 6 com.apple.JavaScriptCore 0x0000000102e465c1 JSC::Heap::markRoots(double, void*, void*, int (&) [37]) + 849 (Heap.cpp:890) 7 com.apple.JavaScriptCore 0x0000000102e48df8 JSC::Heap::collectImpl(JSC::HeapOperation, void*, void*, int (&) [37]) + 600 (memory:2710) 8 com.apple.JavaScriptCore 0x0000000102e48b70 JSC::Heap::collect(JSC::HeapOperation) + 96 (Heap.cpp:1124) 9 com.apple.JavaScriptCore 0x0000000102e3f4cc JSC::GCActivityCallback::doWork() + 76 (GCActivityCallback.cpp:88) 10 com.apple.JavaScriptCore 0x0000000102e4f1a3 JSC::HeapTimer::timerDidFire(__CFRunLoopTimer*, void*) + 179 (HeapTimer.cpp:103) 11 com.apple.CoreFoundation 0x00007fffab6ce364 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 20 12 com.apple.CoreFoundation 0x00007fffab6cdfef __CFRunLoopDoTimer + 1071 13 com.apple.CoreFoundation 0x00007fffab6cdb4a __CFRunLoopDoTimers + 298 14 com.apple.CoreFoundation 0x00007fffab6c5341 __CFRunLoopRun + 2065 15 com.apple.CoreFoundation 0x00007fffab6c48cd CFRunLoopRunSpecific + 285 16 com.apple.WebCore 0x00000001049f2bac WebCore::WorkerRunLoop::runInMode(WebCore::WorkerGlobalScope*, WebCore::ModePredicate const&, WebCore::WorkerRunLoop::WaitMode) + 332 (WorkerRunLoop.cpp:193) 17 com.apple.WebCore 0x00000001049f2e18 WebCore::WorkerRunLoop::runInMode(WebCore::WorkerGlobalScope*, WTF::String const&, WebCore::WorkerRunLoop::WaitMode) + 104 (WorkerRunLoop.cpp:140) 18 com.apple.WebCore 0x00000001049f6930 WebCore::WorkerThreadableLoader::loadResourceSynchronously(WebCore::WorkerGlobalScope*, WebCore::ResourceRequest const&, WebCore::ThreadableLoaderClient&, WebCore::ThreadableLoaderOptions const&) + 208 (RefPtr.h:71) 19 com.apple.WebCore 0x00000001049f4a4d WebCore::WorkerScriptLoader::loadSynchronously(WebCore::ScriptExecutionContext*, WebCore::URL const&, WebCore::CrossOriginRequestPolicy, WebCore::ContentSecurityPolicyEnforcement) + 221 (WorkerScriptLoader.cpp:75) 20 com.apple.WebCore 0x00000001049eee0a WebCore::WorkerGlobalScope::importScripts(WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul> const&, int&) + 490 (WorkerScriptLoader.h:65) 21 com.apple.WebCore 0x0000000103b352c2 WebCore::DedicatedWorkerGlobalScope::importScripts(WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul> const&, int&) + 18 (WorkerGlobalScope.h:87) 22 com.apple.WebCore 0x000000010431163d WebCore::JSWorkerGlobalScope::importScripts(JSC::ExecState&) + 301 (JSWorkerGlobalScopeCustom.cpp:76) 23 com.apple.WebCore 0x0000000104310a0c WebCore::jsWorkerGlobalScopeInstanceFunctionImportScripts(JSC::ExecState*) + 140 (JSWorkerGlobalScope.cpp:1585) 24 ??? 0x00004d340e603ca8 0 + 84885974826152 25 com.apple.JavaScriptCore 0x0000000103092d87 llint_entry + 24665 (LowLevelInterpreter.asm:718) 26 com.apple.JavaScriptCore 0x000000010308cb4b vmEntryToJavaScript + 299 (LowLevelInterpreter64.asm:251) 27 com.apple.JavaScriptCore 0x0000000102f048ce JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 158 (JITCode.cpp:81) 28 com.apple.JavaScriptCore 0x0000000102eb8516 JSC::Interpreter::execute(JSC::ProgramExecutable*, JSC::ExecState*, JSC::JSObject*) + 15334 (Interpreter.cpp:955) 29 com.apple.JavaScriptCore 0x0000000102b0d925 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&) + 469 (Completion.cpp:107) 30 com.apple.WebCore 0x00000001049f4458 WebCore::WorkerScriptController::evaluate(WebCore::ScriptSourceCode const&, WTF::NakedPtr<JSC::Exception>&) + 120 (MarkedBlock.h:235) 31 com.apple.WebCore 0x00000001049f4386 WebCore::WorkerScriptController::evaluate(WebCore::ScriptSourceCode const&) + 38 (WorkerScriptController.cpp:115) 32 com.apple.WebCore 0x00000001049f5a66 WebCore::WorkerThread::workerThread() + 534 (utility:753) 33 com.apple.JavaScriptCore 0x00000001032f17c2 WTF::threadEntryPoint(void*) + 178 (functional:1766) 34 com.apple.JavaScriptCore 0x00000001032f1b6f WTF::wtfThreadEntryPoint(void*) + 15 (memory:2723) 35 libsystem_pthread.dylib 0x00007fffbf9f7c2b _pthread_body + 176 36 libsystem_pthread.dylib 0x00007fffbf9f7b7b _pthread_start + 274 37 libsystem_pthread.dylib 0x00007fffbf9f73bd thread_start + 13

Comment on attachment 280717 [details] Initial attempt (not working) View in context: https://bugs.webkit.org/attachment.cgi?id=280717&action=review > Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.h:69 > + JSC::Strong<JSC::JSProxy> m_proxy; You'd have to change this to a visited GC pointer to avoid a leak.

(In reply to comment #11) > Comment on attachment 280717 [details] > Initial attempt (not working) > > View in context: > https://bugs.webkit.org/attachment.cgi?id=280717&action=review > > > Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.h:69 > > + JSC::Strong<JSC::JSProxy> m_proxy; > > You'd have to change this to a visited GC pointer to avoid a leak. Sure, what's the type/class of a GC pointer? (sorry, not familiar with JSC types)

(In reply to comment #12) > (In reply to comment #11) > > Comment on attachment 280717 [details] > > Initial attempt (not working) > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=280717&action=review > > > > > Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.h:69 > > > + JSC::Strong<JSC::JSProxy> m_proxy; > > > > You'd have to change this to a visited GC pointer to avoid a leak. > > Sure, what's the type/class of a GC pointer? (sorry, not familiar with JSC > types) Never mind, Mark pointed me to WriteBarrier on IRC. I'll update the patch.

I think you need to set JSGlobalObject::setGlobalThis to the proxy as well. See how JSDOMWindow does this. The call to jsWorkerGlobalScopeInstanceFunctionImportScripts probably has the wrong 'this' value, causing toJSDedicatedWorkerGlobalScope to return nullptr. BTW, it seems strange that toJSDedicatedWorkerGlobalScope returns nullptr instead of crashing. When is it OK to nullptr?

Created attachment 280728 [details] Initial patch (not working)

Thread 15 Crashed:: WebCore: Worker 0 com.apple.JavaScriptCore 0x0000000106ac2c14 WTFCrash + 36 (Assertions.cpp:317) 1 com.apple.WebCore 0x0000000108f5c7a7 void* JSC::allocateCell<JSC::Structure>(JSC::Heap&, unsigned long) + 215 (JSCellInlines.h:135) 2 com.apple.WebCore 0x0000000108f5c5dc void* JSC::allocateCell<JSC::Structure>(JSC::Heap&) + 28 (JSCellInlines.h:145) 3 com.apple.WebCore 0x0000000108f5c48c JSC::Structure::create(JSC::VM&, JSC::JSGlobalObject*, JSC::JSValue, JSC::TypeInfo const&, JSC::ClassInfo const*, unsigned char, unsigned int) + 188 (StructureInlines.h:42) 4 com.apple.WebCore 0x000000010a4050c3 WebCore::JSWorkerGlobalScopeBase::finishCreation(JSC::VM&) + 99 (JSWorkerGlobalScopeBase.cpp:58) 5 com.apple.WebCore 0x000000010a3fe37d WebCore::JSWorkerGlobalScope::finishCreation(JSC::VM&) + 45 (JSWorkerGlobalScope.cpp:401) 6 com.apple.WebCore 0x0000000109df84e8 WebCore::JSDedicatedWorkerGlobalScope::finishCreation(JSC::VM&) + 40 (JSDedicatedWorkerGlobalScope.cpp:114) 7 com.apple.WebCore 0x000000010b34e21b WebCore::JSDedicatedWorkerGlobalScope::create(JSC::VM&, JSC::Structure*, WTF::Ref<WebCore::DedicatedWorkerGlobalScope>&&) + 123 (JSDedicatedWorkerGlobalScope.h:36) 8 com.apple.WebCore 0x000000010b34d612 WebCore::WorkerScriptController::initScript() + 642 (WorkerScriptController.cpp:92) 9 com.apple.WebCore 0x0000000109e320f9 WebCore::WorkerScriptController::initScriptIfNeeded() + 57 (WorkerScriptController.h:94) 10 com.apple.WebCore 0x000000010b34e502 WebCore::WorkerScriptController::evaluate(WebCore::ScriptSourceCode const&, WTF::NakedPtr<JSC::Exception>&) + 66 (WorkerScriptController.cpp:128) 11 com.apple.WebCore 0x000000010b34e3cc WebCore::WorkerScriptController::evaluate(WebCore::ScriptSourceCode const&) + 76 (WorkerScriptController.cpp:114) 12 com.apple.WebCore 0x000000010b3515d5 WebCore::WorkerThread::workerThread() + 853 (WorkerThread.cpp:166) 13 com.apple.WebCore 0x000000010b351275 WebCore::WorkerThread::workerThreadStart(void*) + 21 (WorkerThread.cpp:140) 14 com.apple.JavaScriptCore 0x0000000106b21f29 WTF::createThread(void (*)(void*), void*, char const*)::$_0::operator()() const + 25 (Threading.cpp:84) 15 com.apple.JavaScriptCore 0x0000000106b21efd void std::__1::__invoke_void_return_wrapper<void>::__call<WTF::createThread(void (*)(void*), void*, char const*)::$_0&>(WTF::createThread(void (*)(void*), void*, char const*)::$_0&&&) + 45 (__functional_base:469) 16 com.apple.JavaScriptCore 0x0000000106b21ea9 std::__1::__function::__func<WTF::createThread(void (*)(void*), void*, char const*)::$_0, std::__1::allocator<WTF::createThread(void (*)(void*), void*, char const*)::$_0>, void ()>::operator()() + 41 (functional:1437) 17 com.apple.JavaScriptCore 0x000000010617e43a std::__1::function<void ()>::operator()() const + 26 (functional:1817) 18 com.apple.JavaScriptCore 0x0000000106b20b47 WTF::threadEntryPoint(void*) + 151 (Threading.cpp:60) 19 com.apple.JavaScriptCore 0x0000000106b22511 WTF::wtfThreadEntryPoint(void*) + 289 (ThreadingPthreads.cpp:164) 20 libsystem_pthread.dylib 0x00007fffbf9f7c2b _pthread_body + 176 21 libsystem_pthread.dylib 0x00007fffbf9f7b7b _pthread_start + 274 22 libsystem_pthread.dylib 0x00007fffbf9f73bd thread_start + 13

(In reply to comment #16) > Thread 15 Crashed:: WebCore: Worker > 0 com.apple.JavaScriptCore 0x0000000106ac2c14 WTFCrash + 36 > (Assertions.cpp:317) > 1 com.apple.WebCore 0x0000000108f5c7a7 void* > JSC::allocateCell<JSC::Structure>(JSC::Heap&, unsigned long) + 215 > (JSCellInlines.h:135) > 2 com.apple.WebCore 0x0000000108f5c5dc void* > JSC::allocateCell<JSC::Structure>(JSC::Heap&) + 28 (JSCellInlines.h:145) > 3 com.apple.WebCore 0x0000000108f5c48c > JSC::Structure::create(JSC::VM&, JSC::JSGlobalObject*, JSC::JSValue, > JSC::TypeInfo const&, JSC::ClassInfo const*, unsigned char, unsigned int) + > 188 (StructureInlines.h:42) > 4 com.apple.WebCore 0x000000010a4050c3 > WebCore::JSWorkerGlobalScopeBase::finishCreation(JSC::VM&) + 99 > (JSWorkerGlobalScopeBase.cpp:58) > 5 com.apple.WebCore 0x000000010a3fe37d > WebCore::JSWorkerGlobalScope::finishCreation(JSC::VM&) + 45 > (JSWorkerGlobalScope.cpp:401) > 6 com.apple.WebCore 0x0000000109df84e8 > WebCore::JSDedicatedWorkerGlobalScope::finishCreation(JSC::VM&) + 40 > (JSDedicatedWorkerGlobalScope.cpp:114) > 7 com.apple.WebCore 0x000000010b34e21b > WebCore::JSDedicatedWorkerGlobalScope::create(JSC::VM&, JSC::Structure*, > WTF::Ref<WebCore::DedicatedWorkerGlobalScope>&&) + 123 > (JSDedicatedWorkerGlobalScope.h:36) > 8 com.apple.WebCore 0x000000010b34d612 > WebCore::WorkerScriptController::initScript() + 642 > (WorkerScriptController.cpp:92) > 9 com.apple.WebCore 0x0000000109e320f9 > WebCore::WorkerScriptController::initScriptIfNeeded() + 57 > (WorkerScriptController.h:94) > 10 com.apple.WebCore 0x000000010b34e502 > WebCore::WorkerScriptController::evaluate(WebCore::ScriptSourceCode const&, > WTF::NakedPtr<JSC::Exception>&) + 66 (WorkerScriptController.cpp:128) > 11 com.apple.WebCore 0x000000010b34e3cc > WebCore::WorkerScriptController::evaluate(WebCore::ScriptSourceCode const&) > + 76 (WorkerScriptController.cpp:114) > 12 com.apple.WebCore 0x000000010b3515d5 > WebCore::WorkerThread::workerThread() + 853 (WorkerThread.cpp:166) > 13 com.apple.WebCore 0x000000010b351275 > WebCore::WorkerThread::workerThreadStart(void*) + 21 (WorkerThread.cpp:140) > 14 com.apple.JavaScriptCore 0x0000000106b21f29 WTF::createThread(void > (*)(void*), void*, char const*)::$_0::operator()() const + 25 > (Threading.cpp:84) > 15 com.apple.JavaScriptCore 0x0000000106b21efd void > std::__1::__invoke_void_return_wrapper<void>::__call<WTF::createThread(void > (*)(void*), void*, char const*)::$_0&>(WTF::createThread(void (*)(void*), > void*, char const*)::$_0&&&) + 45 (__functional_base:469) > 16 com.apple.JavaScriptCore 0x0000000106b21ea9 > std::__1::__function::__func<WTF::createThread(void (*)(void*), void*, char > const*)::$_0, std::__1::allocator<WTF::createThread(void (*)(void*), void*, > char const*)::$_0>, void ()>::operator()() + 41 (functional:1437) > 17 com.apple.JavaScriptCore 0x000000010617e43a > std::__1::function<void ()>::operator()() const + 26 (functional:1817) > 18 com.apple.JavaScriptCore 0x0000000106b20b47 > WTF::threadEntryPoint(void*) + 151 (Threading.cpp:60) > 19 com.apple.JavaScriptCore 0x0000000106b22511 > WTF::wtfThreadEntryPoint(void*) + 289 (ThreadingPthreads.cpp:164) > 20 libsystem_pthread.dylib 0x00007fffbf9f7c2b _pthread_body + 176 > 21 libsystem_pthread.dylib 0x00007fffbf9f7b7b _pthread_start + 274 > 22 libsystem_pthread.dylib 0x00007fffbf9f73bd thread_start + 13 Ok, the issue seems to be that I cannot call JSC::Structure::create() from inside finishCreation().

Created attachment 280734 [details] WIP Patch

With this latest patch, there are no more crashes but workers are very broken. E.g. 'DedicatedWorkerGlobalScope' in self // returns false

Is DedicatedWorkerGlobalScope a property of DedicatedWorkerGlobalScope?

(In reply to comment #20) > Is DedicatedWorkerGlobalScope a property of DedicatedWorkerGlobalScope? Yes, it is.

(In reply to comment #21) > (In reply to comment #20) > > Is DedicatedWorkerGlobalScope a property of DedicatedWorkerGlobalScope? > > Yes, it is. static const HashTableValue JSDedicatedWorkerGlobalScopeTableValues[] = { { "onmessage", CustomAccessor, NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsDedicatedWorkerGlobalScopeOnmessage), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSDedicatedWorkerGlobalScopeOnmessage) } }, { "DedicatedWorkerGlobalScope", DontEnum, NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsDedicatedWorkerGlobalScopeDedicatedWorkerGlobalScopeConstructor), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSDedicatedWorkerGlobalScopeDedicatedWorkerGlobalScopeConstructor) } }, { "postMessage", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsDedicatedWorkerGlobalScopeInstanceFunctionPostMessage), (intptr_t) (1) } }, }; In WebKitBuild/Debug/DerivedSources/WebCore/JSDedicatedWorkerGlobalScope.cpp

Comment on attachment 280734 [details] WIP Patch View in context: https://bugs.webkit.org/attachment.cgi?id=280734&action=review > Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp:64 > + auto* structure = Structure::create(vm, 0, jsNull(), TypeInfo(PureForwardingProxyType, JSProxy::StructureFlags), JSProxy::info()); I have tried replacing this line with: auto* structure = JSProxy::createStructure(vm, this, jsNull(), PureForwardingProxyType); So that the globalObject is set to this on the JSProxy but this did not help.

Mark suggested something. I am in the process of trying it out.

Created attachment 280742 [details] WIP Patch New work-in-progress patch. It is not pretty but it seems to work. The pass rates on W3C test suites are way up: http://w3c-test.org/workers/interfaces.worker: 20 passes -> 50 passes (out of 128) http://w3c-test.org/IndexedDB/interfaces.worker 0 passes -> 145 passes (out of 156)

Wow, looks like the layout tests are even passing. I'll add some testing and changelogs.

Created attachment 280745 [details] Patch

Comment on attachment 280745 [details] Patch Attachment 280745 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1462570 Number of test failures exceeded the failure limit.

Created attachment 280748 [details] Archive of layout-test-results from ews112 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-yosemite Platform: Mac OS X 10.10.5

Created attachment 280754 [details] Patch

*** Bug 157600 has been marked as a duplicate of this bug. ***

Comment on attachment 280754 [details] Patch Seems great.

Comment on attachment 280754 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=280754&action=review > Source/WebCore/bindings/js/JSWorkerGlobalScopeCustom.cpp:104 > + return proxy(); I don't even think I need these custom bindings anymore since toJS() will return the proxy. I'll try it out.

Created attachment 280808 [details] Patch

Comment on attachment 280808 [details] Patch r=me

Comment on attachment 280808 [details] Patch Clearing flags on attachment: 280808 Committed r201808: <http://trac.webkit.org/changeset/201808>

All reviewed patches have been landed. Closing bug.