Closed
Bug 1113340
Opened 10 years ago
Closed 10 years ago
XHR read of (IndexedDB) Blob from worker thread deadlocks main thread
Categories
(Core :: Storage: IndexedDB, defect)
Tracking
()
RESOLVED
FIXED
mozilla37
Tracking | Status | |
---|---|---|
firefox36 | --- | unaffected |
firefox37 | --- | fixed |
People
(Reporter: asuth, Assigned: bent.mozilla)
References
Details
Attachments
(1 file, 2 obsolete files)
7.30 KB,
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
If I do an XHR against a Blob via URL.CreateObjectURL from a worker, the main thread deadlocks with a stack like this (with an hg tip of 9bb8b0b4daae): 0 pthread_cond_wait@@GLIBC_2.3.2 nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185 1 PR_WaitCondVar nsprpub/pr/src/pthreads/ptsynch.c:385 2 Wait ../dist/include/mozilla/CondVar.h:79 3 Wait ../dist/include/mozilla/Monitor.h:40 4 Wait ../dist/include/mozilla/Monitor.h:88 5 dom::BlobChild::RemoteBlobImpl::CreateStreamHelper::GetStream dom/ipc/Blob.cpp:2472 6 dom::BlobChild::RemoteBlobImpl::GetInternalStream dom/ipc/Blob.cpp:2360 7 nsHostObjectProtocolHandler::NewChannel2 dom/base/nsHostObjectProtocolHandler.cpp:518 8 nsIOService::NewChannelFromURIWithProxyFlagsInternal netwerk/base/src/nsIOService.cpp:669 9 nsIOService::NewChannelFromURIWithProxyFlags2 netwerk/base/src/nsIOService.cpp:758 10 nsIOService::NewChannelFromURI2 netwerk/base/src/nsIOService.cpp:594 11 NS_NewChannelInternal ../dist/include/nsNetUtil.h:289 12 NS_NewChannel ../dist/include/nsNetUtil.h:446 13 nsXMLHttpRequest::Open dom/base/nsXMLHttpRequest.cpp:1764 14 Open dom/workers/../base/nsXMLHttpRequest.h:319 15 MainThreadRunInternal dom/workers/XMLHttpRequest.cpp:1491 16 (anonymous namespace)::OpenRunnable::MainThreadRun dom/workers/XMLHttpRequest.cpp:781 17 (anonymous namespace)::WorkerThreadProxySyncRunnable::Run dom/workers/XMLHttpRequest.cpp:1433 18 nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:835 19 NS_ProcessNextEvent xpcom/glue/nsThreadUtils.cpp:265 20 ipc::MessagePump::Run ipc/glue/MessagePump.cpp:140 21 RunHandler ipc/chromium/src/base/message_loop.cc:226 22 MessageLoop::Run ipc/chromium/src/base/message_loop.cc:200 23 nsBaseAppShell::Run widget/nsBaseAppShell.cpp:164 24 nsAppStartup::Run toolkit/components/startup/nsAppStartup.cpp:281 25 XREMain::XRE_mainRun toolkit/xre/nsAppRunner.cpp:4150 26 XREMain::XRE_main toolkit/xre/nsAppRunner.cpp:4226 27 XRE_main toolkit/xre/nsAppRunner.cpp:4446 28 do_main browser/app/nsBrowserApp.cpp:292 29 main browser/app/nsBrowserApp.cpp:661 This specific stack is from the attached mochitest. This is almost certainly from bug 701634 landing, and this is almost certainly my bad for not having yet gotten the Gaia email backend or frontend tests in a way that shows up in treeherder for try builds, etc. The good news is it's very clear what I should get the IndexedDB team as a present for the holidays...
This will be fun.
Assignee | ||
Comment 2•10 years ago
|
||
Andrew, you're trying to ruin my holidays, aren't you?
Reporter | ||
Comment 3•10 years ago
|
||
something something @RealAndreasGal something something "enhanced" holidays? Less edgily, I reallllly thought we had coverage for this in dom/indexedDB already. But test_blob_simple.html only does main-thread XHR and its worker read is FileReaderSync.
Assignee | ||
Comment 4•10 years ago
|
||
DEBUG builds assert at least...
Assignee | ||
Comment 5•10 years ago
|
||
Thanks for the test!
Assignee: nobody → bent.mozilla
Attachment #8538728 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8538894 -
Flags: review?(khuey)
Attachment #8538894 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 6•10 years ago
|
||
We can optimize it a little more and avoid creating blob actors when we don't need them.
Attachment #8538894 -
Attachment is obsolete: true
Attachment #8538935 -
Flags: review?(khuey)
Attachment #8538935 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 7•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/daa88419ad98
Comment 8•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/daa88419ad98
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Assignee | ||
Comment 9•9 years ago
|
||
Comment on attachment 8538935 [details] [diff] [review] Patch, v1.1 Approval Request Comment (Please see bug 1120336 comment 8)
Attachment #8538935 -
Flags: approval-mozilla-beta?
Attachment #8538935 -
Flags: approval-mozilla-aurora?
Comment 10•9 years ago
|
||
Comment on attachment 8538935 [details] [diff] [review] Patch, v1.1 This is already on Aurora.
Attachment #8538935 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
status-firefox36:
--- → affected
status-firefox37:
--- → fixed
Assignee | ||
Comment 11•9 years ago
|
||
Er, oops. Thanks!
Updated•9 years ago
|
Attachment #8538935 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 13•9 years ago
|
||
Per an IRC convo with bent, this isn't needed after all because bug 701634 didn't land on Gecko 36. At his request, I've reverted the code changes but left the tests in place. https://hg.mozilla.org/releases/mozilla-beta/rev/e8effa80da5b
Updated•9 years ago
|
Attachment #8538935 -
Flags: approval-mozilla-beta+
You need to log in
before you can comment on or make changes to this bug.
Description
•