XHR read of (IndexedDB) Blob from worker thread deadlocks main thread

RESOLVED FIXED in Firefox 37

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: asuth, Assigned: bent.mozilla)

Tracking

Trunk
mozilla37
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox36 unaffected, firefox37 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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...
Andrew, you're trying to ruin my holidays, aren't you?
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.
DEBUG builds assert at least...
Posted patch Patch, v1 (obsolete) — Splinter Review
Thanks for the test!
Assignee: nobody → bent.mozilla
Attachment #8538728 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8538894 - Flags: review?(khuey)
Posted patch Patch, v1.1Splinter Review
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)
https://hg.mozilla.org/mozilla-central/rev/daa88419ad98
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
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 on attachment 8538935 [details] [diff] [review]
Patch, v1.1

This is already on Aurora.
Attachment #8538935 - Flags: approval-mozilla-aurora?
Attachment #8538935 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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
Attachment #8538935 - Flags: approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.