Bug 179062 – http/tests/workers/service/ServiceWorkerGlobalScope-properties.html is flaky

• Component: WebCore Misc.
• Status: RESOLVED DUPLICATE

[Chris Dumez]

http/tests/workers/service/ServiceWorkerGlobalScope-properties.html is a flaky timeout and sometimes crashes:
Process:               com.apple.WebKit.WebContent.Development [82928]
Path:                  /Volumes/VOLUME/*/WebKit.framework/Versions/A/XPCServices/com.apple.WebKit.WebContent.xpc/Contents/MacOS/com.apple.WebKit.WebContent.Development
Identifier:            com.apple.WebKit.WebContent
Version:               605+ (605.1.3+)
Code Type:             X86-64 (Native)
Parent Process:        ??? [1]
Responsible:           WebKitTestRunner [82844]
User ID:               501

Date/Time:             2017-10-31 08:38:03.516 -0700
OS Version:            Mac OS X 10.11.6 (15G1510)
Report Version:        11
Anonymous UUID:        672D1B3C-38BC-D2F3-E02E-5DA86382E04A


Time Awake Since Boot: 510000 seconds

System Integrity Protection: enabled

Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
Exception Codes:       KERN_INVALID_ADDRESS at 0x00000000bbadbeef
Exception Note:        EXC_CORPSE_NOTIFY

VM Regions Near 0xbbadbeef:
--> 
    __TEXT                 0000000100feb000-0000000100fee000 [   12K] r-x/rwx SM=COW  /Volumes/VOLUME/*/WebKit.framework/Versions/A/XPCServices/com.apple.WebKit.WebContent.xpc/Contents/MacOS/com.apple.WebKit.WebContent.Development

Application Specific Information:
CRASHING TEST: /workers/service/ServiceWorkerGlobalScope-properties.html

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x0000000114268be7 WTFCrash + 39 (Assertions.cpp:270)
1   com.apple.WebCore             	0x0000000106d081d8 WebCore::JSEventListener::jsFunction(WebCore::ScriptExecutionContext&) const + 424 (JSEventListener.h:128)
2   com.apple.WebCore             	0x0000000106d04e3d WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&) + 205 (JSEventListener.cpp:97)
3   com.apple.WebCore             	0x0000000107254b6e WebCore::EventTarget::fireEventListeners(WebCore::Event&, WTF::Vector<WTF::RefPtr<WebCore::RegisteredEventListener>, 1ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>) + 974 (EventTarget.cpp:298)
4   com.apple.WebCore             	0x0000000107251f36 WebCore::EventTarget::fireEventListeners(WebCore::Event&) + 326 (EventTarget.cpp:238)
5   com.apple.WebCore             	0x0000000107254769 WebCore::EventTarget::dispatchEvent(WebCore::Event&) + 233 (EventTarget.cpp:195)
6   com.apple.WebCore             	0x0000000108591c80 WebCore::SWClientConnection::postMessageToServiceWorkerClient(unsigned long long, WTF::Ref<WebCore::SerializedScriptValue>&&, unsigned long long, WTF::String const&) + 624 (SWClientConnection.cpp:137)
7   com.apple.WebKit              	0x0000000101c73cb5 WebKit::WebSWClientConnection::postMessageToServiceWorkerClient(unsigned long long, IPC::DataReference const&, unsigned long long, WTF::String const&) + 101 (WebSWClientConnection.cpp:102)
8   com.apple.WebKit              	0x0000000101c7a717 void IPC::callMemberFunctionImpl<WebKit::WebSWClientConnection, void (WebKit::WebSWClientConnection::*)(unsigned long long, IPC::DataReference const&, unsigned long long, WTF::String const&), std::__1::tuple<unsigned long long, IPC::DataReference, unsigned long long, WTF::String>, 0ul, 1ul, 2ul, 3ul>(WebKit::WebSWClientConnection*, void (WebKit::WebSWClientConnection::*)(unsigned long long, IPC::DataReference const&, unsigned long long, WTF::String const&), std::__1::tuple<unsigned long long, IPC::DataReference, unsigned long long, WTF::String>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul>) + 327 (HandleMessage.h:41)
9   com.apple.WebKit              	0x0000000101c7a338 void IPC::callMemberFunction<WebKit::WebSWClientConnection, void (WebKit::WebSWClientConnection::*)(unsigned long long, IPC::DataReference const&, unsigned long long, WTF::String const&), std::__1::tuple<unsigned long long, IPC::DataReference, unsigned long long, WTF::String>, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul> >(std::__1::tuple<unsigned long long, IPC::DataReference, unsigned long long, WTF::String>&&, WebKit::WebSWClientConnection*, void (WebKit::WebSWClientConnection::*)(unsigned long long, IPC::DataReference const&, unsigned long long, WTF::String const&)) + 88 (HandleMessage.h:47)
10  com.apple.WebKit              	0x0000000101c7866b void IPC::handleMessage<Messages::WebSWClientConnection::PostMessageToServiceWorkerClient, WebKit::WebSWClientConnection, void (WebKit::WebSWClientConnection::*)(unsigned long long, IPC::DataReference const&, unsigned long long, WTF::String const&)>(IPC::Decoder&, WebKit::WebSWClientConnection*, void (WebKit::WebSWClientConnection::*)(unsigned long long, IPC::DataReference const&, unsigned long long, WTF::String const&)) + 347 (HandleMessage.h:127)
11  com.apple.WebKit              	0x0000000101c77da2 WebKit::WebSWClientConnection::didReceiveMessage(IPC::Connection&, IPC::Decoder&) + 770 (WebSWClientConnectionMessageReceiver.cpp:68)
12  com.apple.WebKit              	0x0000000101c8fbaf WebKit::WebToStorageProcessConnection::didReceiveMessage(IPC::Connection&, IPC::Decoder&) + 319 (WebToStorageProcessConnection.cpp:70)
13  com.apple.WebKit              	0x000000010113d393 IPC::Connection::dispatchMessage(IPC::Decoder&) + 51 (Connection.cpp:902)
14  com.apple.WebKit              	0x0000000101134651 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) + 721 (Connection.cpp:930)
15  com.apple.WebKit              	0x000000010113d99f IPC::Connection::dispatchOneMessage() + 1535 (Connection.cpp:959)
16  com.apple.WebKit              	0x00000001011449cd IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14::operator()() + 29 (Connection.cpp:896)
17  com.apple.WebKit              	0x000000010114491c WTF::Function<void ()>::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14>::call() + 28 (Function.h:101)
18  com.apple.JavaScriptCore      	0x00000001142a6aee WTF::Function<void ()>::operator()() const + 142 (Function.h:56)
19  com.apple.JavaScriptCore      	0x00000001142c706b WTF::RunLoop::performWork() + 219 (RunLoop.cpp:107)
20  com.apple.JavaScriptCore      	0x00000001142c7954 WTF::RunLoop::performWork(void*) + 36 (RunLoopCF.cpp:38)
21  com.apple.CoreFoundation      	0x00007fff95b497e1 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
22  com.apple.CoreFoundation      	0x00007fff95b28f0c __CFRunLoopDoSources0 + 556
23  com.apple.CoreFoundation      	0x00007fff95b2842f __CFRunLoopRun + 927
24  com.apple.CoreFoundation      	0x00007fff95b27e28 CFRunLoopRunSpecific + 296
25  com.apple.HIToolbox           	0x00007fff8a4a1935 RunCurrentEventLoopInMode + 235
26  com.apple.HIToolbox           	0x00007fff8a4a176f ReceiveNextEventCommon + 432
27  com.apple.HIToolbox           	0x00007fff8a4a15af _BlockUntilNextEventMatchingListInModeWithFilter + 71
28  com.apple.AppKit              	0x00007fff8b0d4df6 _DPSNextEvent + 1067
29  com.apple.AppKit              	0x00007fff8b0d4226 -[NSApplication _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 454
30  com.apple.AppKit              	0x00007fff8b0c8d80 -[NSApplication run] + 682
31  com.apple.AppKit              	0x00007fff8b092368 NSApplicationMain + 1176
32  libxpc.dylib                  	0x00007fff97ce6194 _xpc_objc_main + 795
33  libxpc.dylib                  	0x00007fff97ce4bbe xpc_main + 494
34  com.apple.WebKit.WebContent   	0x0000000100fecfdd main + 1197
35  libdyld.dylib                 	0x00007fff947915ad start + 1

[Chris Dumez]

The timeouts / crashes seem to go away with the fix for Bug 179067. I'll investigate why.

[Chris Dumez]

(In reply to Chris Dumez from comment #1)
> The timeouts / crashes seem to go away with the fix for Bug 179067. I'll
> investigate why.

The assertion that was hit in JSEventListener::jsFunction() is:
// Verify that we have a valid wrapper protecting our function from
    // garbage collection. That is except for when we're not in the normal
    // world and can have zombie m_jsFunctions.
    ASSERT(!m_isolatedWorld->isNormal() || m_wrapper || !m_jsFunction);

So this seems to indicate that the JSEventListener object's wrapper has been collected too early.

Normally, such collection is prevented by JSEventTarget's visitor:
void JSEventTarget::visitAdditionalChildren(SlotVisitor& visitor)
{
    wrapped().visitJSEventListeners(visitor);
}

JSServiceWorkerContainer subclasses JSEventTargets and should therefore use this visiting code.

[Chris Dumez]

(In reply to Chris Dumez from comment #2)
> (In reply to Chris Dumez from comment #1)
> > The timeouts / crashes seem to go away with the fix for Bug 179067. I'll
> > investigate why.
> 
> The assertion that was hit in JSEventListener::jsFunction() is:
> // Verify that we have a valid wrapper protecting our function from
>     // garbage collection. That is except for when we're not in the normal
>     // world and can have zombie m_jsFunctions.
>     ASSERT(!m_isolatedWorld->isNormal() || m_wrapper || !m_jsFunction);
> 
> So this seems to indicate that the JSEventListener object's wrapper has been
> collected too early.
> 
> Normally, such collection is prevented by JSEventTarget's visitor:
> void JSEventTarget::visitAdditionalChildren(SlotVisitor& visitor)
> {
>     wrapped().visitJSEventListeners(visitor);
> }
> 
> JSServiceWorkerContainer subclasses JSEventTargets and should therefore use
> this visiting code.

void EventTarget::visitJSEventListeners() is implemented like so:

void EventTarget::visitJSEventListeners(JSC::SlotVisitor& visitor)
{
    EventTargetData* data = eventTargetDataConcurrently();
    if (!data)
        return;
    
    auto locker = holdLock(data->eventListenerMap.lock());
    EventListenerIterator iterator(&data->eventListenerMap);
    while (auto* listener = iterator.nextListener())
        listener->visitJSFunction(visitor);
}

[Chris Dumez]

(In reply to Chris Dumez from comment #3)
> (In reply to Chris Dumez from comment #2)
> > (In reply to Chris Dumez from comment #1)
> > > The timeouts / crashes seem to go away with the fix for Bug 179067. I'll
> > > investigate why.
> > 
> > The assertion that was hit in JSEventListener::jsFunction() is:
> > // Verify that we have a valid wrapper protecting our function from
> >     // garbage collection. That is except for when we're not in the normal
> >     // world and can have zombie m_jsFunctions.
> >     ASSERT(!m_isolatedWorld->isNormal() || m_wrapper || !m_jsFunction);
> > 
> > So this seems to indicate that the JSEventListener object's wrapper has been
> > collected too early.
> > 
> > Normally, such collection is prevented by JSEventTarget's visitor:
> > void JSEventTarget::visitAdditionalChildren(SlotVisitor& visitor)
> > {
> >     wrapped().visitJSEventListeners(visitor);
> > }
> > 
> > JSServiceWorkerContainer subclasses JSEventTargets and should therefore use
> > this visiting code.
> 
> void EventTarget::visitJSEventListeners() is implemented like so:
> 
> void EventTarget::visitJSEventListeners(JSC::SlotVisitor& visitor)
> {
>     EventTargetData* data = eventTargetDataConcurrently();
>     if (!data)
>         return;
>     
>     auto locker = holdLock(data->eventListenerMap.lock());
>     EventListenerIterator iterator(&data->eventListenerMap);
>     while (auto* listener = iterator.nextListener())
>         listener->visitJSFunction(visitor);
> }

My bet is that the JSServiceWorkerContainer wrapper gets collected and therefore stops marking its event listeners. However, the ServiceWorkerContainer object is still alive is still tries to fire events. Since event listeners are no longer marked, they may already be collected by the time we try to fire the event.

[Chris Dumez]

(In reply to Chris Dumez from comment #4)
> (In reply to Chris Dumez from comment #3)
> > (In reply to Chris Dumez from comment #2)
> > > (In reply to Chris Dumez from comment #1)
> > > > The timeouts / crashes seem to go away with the fix for Bug 179067. I'll
> > > > investigate why.
> > > 
> > > The assertion that was hit in JSEventListener::jsFunction() is:
> > > // Verify that we have a valid wrapper protecting our function from
> > >     // garbage collection. That is except for when we're not in the normal
> > >     // world and can have zombie m_jsFunctions.
> > >     ASSERT(!m_isolatedWorld->isNormal() || m_wrapper || !m_jsFunction);
> > > 
> > > So this seems to indicate that the JSEventListener object's wrapper has been
> > > collected too early.
> > > 
> > > Normally, such collection is prevented by JSEventTarget's visitor:
> > > void JSEventTarget::visitAdditionalChildren(SlotVisitor& visitor)
> > > {
> > >     wrapped().visitJSEventListeners(visitor);
> > > }
> > > 
> > > JSServiceWorkerContainer subclasses JSEventTargets and should therefore use
> > > this visiting code.
> > 
> > void EventTarget::visitJSEventListeners() is implemented like so:
> > 
> > void EventTarget::visitJSEventListeners(JSC::SlotVisitor& visitor)
> > {
> >     EventTargetData* data = eventTargetDataConcurrently();
> >     if (!data)
> >         return;
> >     
> >     auto locker = holdLock(data->eventListenerMap.lock());
> >     EventListenerIterator iterator(&data->eventListenerMap);
> >     while (auto* listener = iterator.nextListener())
> >         listener->visitJSFunction(visitor);
> > }
> 
> My bet is that the JSServiceWorkerContainer wrapper gets collected and
> therefore stops marking its event listeners. However, the
> ServiceWorkerContainer object is still alive is still tries to fire events.
> Since event listeners are no longer marked, they may already be collected by
> the time we try to fire the event.

If so, it would make sense that Bug 179067 fixes the issue since it makes sure the JSServiceWorkerContainer wrapper does not get garbage collected until its implementation object (owned by Navigator) is destroyed.

[Chris Dumez]

*** This bug has been marked as a duplicate of bug 179067 ***