Bug 180227 – Many imported/w3c/web-platform-tests/service-workers/ test are failing together intermittently

• Component: New Bugs
• Status: RESOLVED FIXED

[Matt Lewis]

Many imported/w3c/web-platform-tests/service-workers/ test are failing together intermittently on High Sierra.

About 80 service worker test failed together and do so intermittently.

I haven't been able to find a regression point.

Example build:

https://build.webkit.org/results/Apple%20High%20Sierra%20Debug%20WK2%20(Tests)/r225336%20(1049)/results.html
https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20WK2%20(Tests)/builds/1049

Example test:
imported/w3c/web-platform-tests/service-workers/service-worker/clients-matchall.https.html

Diff:
--- /Volumes/Data/slave/highsierra-debug-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/service-workers/service-worker/clients-matchall.https-expected.txt
+++ /Volumes/Data/slave/highsierra-debug-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/service-workers/service-worker/clients-matchall.https-actual.txt
@@ -1,4 +1,5 @@
 
+Harness Error (TIMEOUT), message = null
 
-FAIL Test Clients.matchAll() assert_unreached: unexpected rejection: Passing MessagePort objects to postMessage is not yet supported Reached unreachable code
+TIMEOUT Test Clients.matchAll() Test timed out
 

The diffs on most of the test all have a Harness Error (TIMEOUT)

[Chris Dumez]

Service Worker process crash:
Thread 7 Crashed:: WebCore: Worker
0   com.apple.JavaScriptCore      	0x000000016f553744 WTFCrash + 36 (Assertions.cpp:273)
1   com.apple.WebCore             	0x00000001603683c8 JSC::ExceptionScope::assertNoException() + 152
2   com.apple.WebCore             	0x000000016180e5cb WebCore::callFunction(JSC::ExecState&, JSC::JSValue, JSC::JSValue, JSC::ArgList const&) + 283 (JSDOMPromise.cpp:48)
3   com.apple.WebCore             	0x000000016180e3f5 WebCore::DOMPromise::whenSettled(std::__1::function<void ()>&&) + 709 (JSDOMPromise.cpp:69)
4   com.apple.WebCore             	0x0000000163195dc8 WebCore::ExtendableEvent::addExtendLifetimePromise(WTF::Ref<WebCore::DOMPromise>&&) + 136 (ExtendableEvent.cpp:65)
5   com.apple.WebCore             	0x0000000163195d13 WebCore::ExtendableEvent::waitUntil(WTF::Ref<WebCore::DOMPromise>&&) + 275 (ExtendableEvent.cpp:59)
6   com.apple.WebCore             	0x00000001607d0d32 WebCore::jsExtendableEventPrototypeFunctionWaitUntilBody(JSC::ExecState*, WebCore::JSExtendableEvent*, JSC::ThrowScope&) + 322 (JSExtendableEvent.cpp:198)
7   com.apple.WebCore             	0x00000001607c3f4e long long WebCore::IDLOperation<WebCore::JSExtendableEvent>::call<&(WebCore::jsExtendableEventPrototypeFunctionWaitUntilBody(JSC::ExecState*, WebCore::JSExtendableEvent*, JSC::ThrowScope&)), (WebCore::CastedThisErrorBehavior)0>(JSC::ExecState&, char const*) + 606 (JSDOMOperation.h:53)
8   com.apple.WebCore             	0x00000001607c3cdc WebCore::jsExtendableEventPrototypeFunctionWaitUntil(JSC::ExecState*) + 28 (JSExtendableEvent.cpp:204)
9   ???                           	0x00002173dc4012a8 0 + 36781500142248
10  com.apple.JavaScriptCore      	0x000000016e0f8f4e llint_entry + 31992
11  com.apple.JavaScriptCore      	0x000000016e0f1037 vmEntryToJavaScript + 343
12  com.apple.JavaScriptCore      	0x000000016ee2309e JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 350 (JITCode.cpp:81)
13  com.apple.JavaScriptCore      	0x000000016edca675 JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 1269 (Interpreter.cpp:999)
14  com.apple.JavaScriptCore      	0x000000016f020fda JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 202 (CallData.cpp:41)
15  com.apple.JavaScriptCore      	0x000000016f0210b9 JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) + 201 (CallData.cpp:48)
16  com.apple.JavaScriptCore      	0x000000016f02135d JSC::profiledCall(JSC::ExecState*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) + 125 (CallData.cpp:67)
17  com.apple.WebCore             	0x0000000161827d0c WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&) + 2012 (JSEventListener.cpp:154)
18  com.apple.WebCore             	0x0000000161d778f2 WebCore::EventTarget::fireEventListeners(WebCore::Event&, WTF::Vector<WTF::RefPtr<WebCore::RegisteredEventListener>, 1ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>) + 962 (EventTarget.cpp:287)
19  com.apple.WebCore             	0x0000000161d6f3ca WebCore::EventTarget::fireEventListeners(WebCore::Event&) + 314 (EventTarget.cpp:228)
20  com.apple.WebCore             	0x0000000161d774a9 WebCore::EventTarget::dispatchEvent(WebCore::Event&) + 233 (EventTarget.cpp:188)
21  com.apple.WebCore             	0x00000001631c6d83 WebCore::ServiceWorkerThread::fireInstallEvent()::$_8::operator()(WebCore::ScriptExecutionContext&) + 115 (ServiceWorkerThread.cpp:127)
22  com.apple.WebCore             	0x00000001631c6c94 WTF::Function<void (WebCore::ScriptExecutionContext&)>::CallableWrapper<WebCore::ServiceWorkerThread::fireInstallEvent()::$_8>::call(WebCore::ScriptExecutionContext&) + 52 (Function.h:101)
23  com.apple.WebCore             	0x00000001616f79de WTF::Function<void (WebCore::ScriptExecutionContext&)>::operator()(WebCore::ScriptExecutionContext&) const + 158 (Function.h:56)
24  com.apple.WebCore             	0x00000001616e518d WebCore::ScriptExecutionContext::Task::performTask(WebCore::ScriptExecutionContext&) + 29 (ScriptExecutionContext.h:184)
25  com.apple.WebCore             	0x0000000163184c90 WebCore::WorkerRunLoop::Task::performTask(WebCore::WorkerGlobalScope*) + 128 (WorkerRunLoop.cpp:259)
26  com.apple.WebCore             	0x0000000163183cc7 WebCore::WorkerRunLoop::runInMode(WebCore::WorkerGlobalScope*, WebCore::ModePredicate const&, WebCore::WorkerRunLoop::WaitMode) + 1399 (WorkerRunLoop.cpp:203)
27  com.apple.WebCore             	0x00000001631836b6 WebCore::WorkerRunLoop::run(WebCore::WorkerGlobalScope*) + 86 (WorkerRunLoop.cpp:138)
28  com.apple.WebCore             	0x0000000163187e63 WebCore::WorkerThread::runEventLoop() + 51 (WorkerThread.cpp:258)
29  com.apple.WebCore             	0x00000001631bc2e5 WebCore::ServiceWorkerThread::runEventLoop() + 21 (ServiceWorkerThread.cpp:95)
30  com.apple.WebCore             	0x0000000163187a97 WebCore::WorkerThread::workerThread() + 1719 (WorkerThread.cpp:201)
31  com.apple.WebCore             	0x00000001631939a8 WebCore::WorkerThread::start(WTF::Function<void (WTF::String const&)>&&)::$_12::operator()() const + 24 (WorkerThread.cpp:145)
32  com.apple.WebCore             	0x0000000163193969 WTF::Function<void ()>::CallableWrapper<WebCore::WorkerThread::start(WTF::Function<void (WTF::String const&)>&&)::$_12>::call() + 25 (Function.h:101)
33  com.apple.JavaScriptCore      	0x000000016f58cc0b WTF::Function<void ()>::operator()() const + 139 (Function.h:56)
34  com.apple.JavaScriptCore      	0x000000016f5d907f WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) + 351 (Threading.cpp:129)
35  com.apple.JavaScriptCore      	0x000000016f5de755 WTF::wtfThreadEntryPoint(void*) + 21 (ThreadingPthreads.cpp:223)
36  libsystem_pthread.dylib       	0x00007fff5875d6c1 _pthread_body + 340
37  libsystem_pthread.dylib       	0x00007fff5875d56d _pthread_start + 377
38  libsystem_pthread.dylib       	0x00007fff5875cc5d thread_start + 13

[Chris Dumez]

Youenn says he's seen it before and will look into it

[youenn fablet]

Created attachment 328059 [details]
Patch

[youenn fablet]

I was not able to reproduce locally the crash so cannot fully validate that this patch fixes the issue.

That said, given the stack trace, the patch makes sense to me.
Mark, could you review it?

That said, we should really enable the storage process to restart a crashing service worker process.

[Geoffrey Garen]

Comment on attachment 328059 [details]
Patch

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

> Source/WebCore/bindings/js/JSDOMPromise.cpp:73
> +    EXCEPTION_ASSERT_UNUSED(scope, !scope.exception() || isTerminatedExecutionException(vm, scope.exception()));

If you're asserting that the only possible exception is termination, I think you need to update callFunction to do the same. It currently asserts no exception.

[Alexey Proskuryakov]

Comment on attachment 328059 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        No observable change.

I hoped for better :)

[Alexey Proskuryakov]

> +TIMEOUT Test Clients.matchAll() Test timed out

Is it a tools bug that crashes are reported as timeouts?

[youenn fablet]

(In reply to Geoffrey Garen from comment #5)
> Comment on attachment 328059 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=328059&action=review
> 
> > Source/WebCore/bindings/js/JSDOMPromise.cpp:73
> > +    EXCEPTION_ASSERT_UNUSED(scope, !scope.exception() || isTerminatedExecutionException(vm, scope.exception()));
> 
> If you're asserting that the only possible exception is termination, I think
> you need to update callFunction to do the same. It currently asserts no
> exception.

Oh right, it should moved there.

> > Source/WebCore/ChangeLog:8
> > +        No observable change.
> 
> I hoped for better :)

It is difficult to write a test there since we would need to exercise that code path in a worker at the time it gets terminated.

(In reply to Alexey Proskuryakov from comment #7)
> > +TIMEOUT Test Clients.matchAll() Test timed out
> 
> Is it a tools bug that crashes are reported as timeouts?

Service Worker process is crashing in debug mode due to that assertion.
Web process running the actual test will then time out, potentially for one of these reasons:
- some promises might never get rejected (we should fix that if that is the case)
- some further service worker operations will time out since we no longer have a service worker process (we should fix that as well if that is the case)
- some tests are not handling efficiently all rejections/error cases and could fail early but instead, wait to hit the JS setTimeout to end themselves.

Also, if we were to restart the service worker process when crashing, we would get one test timing out, and not 80 tests.

[youenn fablet]

Created attachment 328126 [details]
Patch

[youenn fablet]

Created attachment 328128 [details]
Patch

[Mark Lam]

Comment on attachment 328128 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 328128 [details]
Patch

Clearing flags on attachment: 328128

Committed r225406: <https://trac.webkit.org/changeset/225406>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35802951>

[Ryan Haddad]

Still seeing 70+ service worker tests fail together after this change:

https://build.webkit.org/results/Apple%20Sierra%20Release%20WK2%20(Tests)/r225406%20(6094)/results.html
https://build.webkit.org/results/Apple%20El%20Capitan%20Release%20WK2%20(Tests)/r225406%20(6353)/results.html
https://build.webkit.org/results/Apple%20El%20Capitan%20Release%20WK2%20(Tests)/r225407%20(6354)/results.html

[youenn fablet]

(In reply to Ryan Haddad from comment #15)
> Still seeing 70+ service worker tests fail together after this change:
> 
> https://build.webkit.org/results/Apple%20Sierra%20Release%20WK2%20(Tests)/
> r225406%20(6094)/results.html
> https://build.webkit.org/results/
> Apple%20El%20Capitan%20Release%20WK2%20(Tests)/r225406%20(6353)/results.html
> https://build.webkit.org/results/
> Apple%20El%20Capitan%20Release%20WK2%20(Tests)/r225407%20(6354)/results.html

This is another crash that needs to be fixed at worker cache storage connection level.
I'll upload a patch for it today or Monday.

[youenn fablet]

(In reply to youenn fablet from comment #16)
> (In reply to Ryan Haddad from comment #15)
> > Still seeing 70+ service worker tests fail together after this change:
> > 
> > https://build.webkit.org/results/Apple%20Sierra%20Release%20WK2%20(Tests)/
> > r225406%20(6094)/results.html
> > https://build.webkit.org/results/
> > Apple%20El%20Capitan%20Release%20WK2%20(Tests)/r225406%20(6353)/results.html
> > https://build.webkit.org/results/
> > Apple%20El%20Capitan%20Release%20WK2%20(Tests)/r225407%20(6354)/results.html
> 
> This is another crash that needs to be fixed at worker cache storage
> connection level.
> I'll upload a patch for it today or Monday.

Filed https://bugs.webkit.org/show_bug.cgi?id=180227