Bug 160498 - http/tests/fetch/fetch-in-worker-crash.html is sometimes crashing
Summary: http/tests/fetch/fetch-in-worker-crash.html is sometimes crashing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
Depends on:
Blocks: 151937
  Show dependency treegraph
 
Reported: 2016-08-03 04:54 PDT by youenn fablet
Modified: 2016-08-03 10:16 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.82 KB, patch)
2016-08-03 05:01 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Adding missing early return (3.83 KB, patch)
2016-08-03 09:49 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2016-08-03 04:54:40 PDT
crash log is:

STDERR: 3   0x7f367e5d3e24 WebCore::WorkerThreadableLoader::MainThreadBridge::clearClientWrapper()
STDERR: 4   0x7f367e5d3eb4 WebCore::WorkerThreadableLoader::MainThreadBridge::cancel()
STDERR: 5   0x7f367e2f431b WebCore::ScriptExecutionContext::stopActiveDOMObjects()
STDERR: 6   0x7f367eabe15c
STDERR: 7   0x7f367eaba531 WebCore::WorkerRunLoop::Task::performTask(WebCore::WorkerRunLoop const&, WebCore::WorkerGlobalScope*)
STDERR: 8   0x7f367eaba6c9 WebCore::WorkerRunLoop::runCleanupTasks(WebCore::WorkerGlobalScope*)
STDERR: 9   0x7f367eabad97 WebCore::WorkerRunLoop::run(WebCore::WorkerGlobalScope*)
STDERR: 10  0x7f367eabd315 WebCore::WorkerThread::workerThread()
Comment 1 youenn fablet 2016-08-03 05:01:34 PDT
Created attachment 285218 [details]
Patch
Comment 2 Chris Dumez 2016-08-03 09:01:57 PDT
Comment on attachment 285218 [details]
Patch

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

> Source/WebCore/loader/WorkerThreadableLoader.cpp:152
> +    if (m_workerClientWrapper->done())

This seems odd, if we're done, then presumably we already called didFail() / didFinishLoading(), therefore, the code below should have no effect. Do we want something like this instead?
if (m_workerClientWrapper->done()) {
    clearClientWrapper();
    return;
}
// ... the test of your code ...
Comment 3 youenn fablet 2016-08-03 09:36:13 PDT
*** Bug 160510 has been marked as a duplicate of this bug. ***
Comment 4 youenn fablet 2016-08-03 09:49:28 PDT
Created attachment 285241 [details]
Adding missing early return
Comment 5 Chris Dumez 2016-08-03 09:52:05 PDT
Comment on attachment 285241 [details]
Adding missing early return

r=me
Comment 6 youenn fablet 2016-08-03 09:55:51 PDT
It may be that the second promise may resolve before the end of the test, in which case the test might still be flaky.
Let's see what bots will say.
Comment 7 WebKit Commit Bot 2016-08-03 10:16:32 PDT
Comment on attachment 285241 [details]
Adding missing early return

Clearing flags on attachment: 285241

Committed r204085: <http://trac.webkit.org/changeset/204085>
Comment 8 WebKit Commit Bot 2016-08-03 10:16:36 PDT
All reviewed patches have been landed.  Closing bug.