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

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Andrew Sutherland [:asuth] (he/him)]

Attachment: mochitest that reproduces the failure

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...

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

This will be fun.

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Andrew, you're trying to ruin my holidays, aren't you?

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

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.

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

DEBUG builds assert at least...

Comment 5

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1

Thanks for the test!

Comment 6

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1.1

We can optimize it a little more and avoid creating blob actors when we don't need them.

Comment 7

[Ben Turner (not reading bugmail, use the needinfo flag!)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/daa88419ad98

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/daa88419ad98

Comment 9

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 8538935 [details] [diff] [review]
Patch, v1.1

Approval Request Comment

(Please see bug 1120336 comment 8)

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8538935 [details] [diff] [review]
Patch, v1.1

This is already on Aurora.

Comment 11

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Er, oops. Thanks!

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c16ed656a43b

Comment 13

[Ryan VanderMeulen [:RyanVM]]

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