1122408 - Potential deadlock detected when running mochitest with --nested_oop option

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Kershaw Chang [:kershaw]]

STR:
Just pick a mochietst test case and use mach to run it with --nested_oop option.

You can see the errors below. Both linux and mac build have this problem.
This should be related to the patch in bug 1107009. If I revert the commit, the problem is gone.

###!!! ERROR: Potential deadlock detected:
=== Cyclical dependency starts at
--- Mutex : IndirectLayerTree (currently acquired)
 calling context
  [stack trace unavailable]
=== Cycle completed at
--- Mutex : IndirectLayerTree (currently acquired)
 calling context
  [stack trace unavailable]
###!!! Deadlock may happen NOW!
[Parent 29928] ###!!! ASSERTION: Potential deadlock detected:
Cyclical dependency starts at
Mutex : IndirectLayerTree (currently acquired)
Cycle completed at
Mutex : IndirectLayerTree (currently acquired)
###!!! Deadlock may happen NOW!
: 'Error', file /home/kershaw/work/firefox_git/xpcom/glue/BlockingResourceBase.cpp, line 307
#01: mozilla::OffTheBooksMutex::Lock() (/home/kershaw/work/firefox_git/xpcom/glue/BlockingResourceBase.cpp:382)
#02: mozilla::layers::CompositorParent::NotifyChildCreated(unsigned long const&) (/home/kershaw/work/firefox_git/gfx/layers/ipc/CompositorParent.cpp:1259)
#03: mozilla::layers::CrossProcessCompositorParent::RecvNotifyChildCreated(unsigned long const&) (/home/kershaw/work/firefox_git/gfx/layers/ipc/CompositorParent.cpp:1645)
#04: mozilla::layers::PCompositorParent::OnMessageReceived(IPC::Message const&) (/home/kershaw/work/firefox_git/obj-x86_64-unknown-linux-gnu/ipc/ipdl/PCompositorParent.cpp:473)
#05: ~AutoSetValue (/home/kershaw/work/firefox_git/obj-x86_64-unknown-linux-gnu/ipc/glue/../../dist/include/mozilla/ipc/MessageChannel.h:490)
#06: mozilla::Maybe<mozilla::dom::AutoNoJSAPI>::reset() (/home/kershaw/work/firefox_git/obj-x86_64-unknown-linux-gnu/ipc/glue/../../dist/include/mozilla/Maybe.h:375 (discriminator 1))
#07: mozilla::ipc::MessageChannel::OnMaybeDequeueOne() (/home/kershaw/work/firefox_git/ipc/glue/MessageChannel.cpp:1129)
#08: MessageLoop::RunTask(Task*) (/home/kershaw/work/firefox_git/ipc/chromium/src/base/message_loop.cc:362)
#09: MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) (/home/kershaw/work/firefox_git/ipc/chromium/src/base/message_loop.cc:372)
#10: MessageLoop::DoWork() (/home/kershaw/work/firefox_git/ipc/chromium/src/base/message_loop.cc:447)
#11: base::MessagePumpDefault::Run(base::MessagePump::Delegate*) (/home/kershaw/work/firefox_git/ipc/chromium/src/base/message_pump_default.cc:35)
#12: MessageLoop::RunInternal() (/home/kershaw/work/firefox_git/ipc/chromium/src/base/message_loop.cc:234)
#13: ~AutoRunState (/home/kershaw/work/firefox_git/ipc/chromium/src/base/message_loop.cc:508)
#14: base::Thread::ThreadMain() (/home/kershaw/work/firefox_git/ipc/chromium/src/base/thread.cc:173)
#15: ThreadFunc (/home/kershaw/work/firefox_git/ipc/chromium/src/base/platform_thread_posix.cc:41)
#16: start_thread (/build/buildd/eglibc-2.19/nptl/pthread_create.c:312 (discriminator 2))
#17: __clone (/build/buildd/eglibc-2.19/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:113)

Comment 1

[Kartikaya Gupta (email:[e-mail])]

From bug 1038620 comment 30, STR are:

./mach mochitest-plain --nested_oop dom/base/test/test_Image_constructor.html

I can reproduce running this on a desktop Linux build, will investigate.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

The problem is that the line of code at [1] is invoked while holding the sIndirectLayerTreesLock (at the start of the function) but the function being invoked there also tries to grab the lock. It's not a re-entrant lock so it self-deadlocks. Pushing the lock out to the callers of CompositorParent::NotifyChildCreated fixes the problem. Patch coming.

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/CompositorParent.cpp?rev=5680121a4909#1645

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

Comment 4

[Benoit Girard (:BenWa)]

Comment on attachment 8550316 [details] [diff] [review]
Patch

Review of attachment 8550316 [details] [diff] [review]:
-----------------------------------------------------------------

Lock all of the things!

Comment 5

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6396e2e43cdd

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8550316 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: bug 1107009 (also pending uplift approval)
[User impact if declined]: possible deadlock in some cases. it was encountered during test conditions but presumably it can be hit in real-world use cases too.
[Describe test coverage new/current, TBPL]: locally and via test harness
[Risks and why]: pretty low risk; almost equivalent to a small refactoring
[String/UUID change made/needed]: none

(Note: this should be uplifted to all branches that bug 1107009 is approved for)

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/6396e2e43cdd

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/98f7ecee1b7a
https://hg.mozilla.org/releases/mozilla-beta/rev/e6df6527d52e
https://hg.mozilla.org/releases/mozilla-esr31/rev/c4c37781a802

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Folded in with the B2G uplifts of bug 1107009.

Comment 10

[Kamil Jozwiak [:kjozwiak]]

Reproduced the issue using the c1f6345f2803 m-c changeset and the instructions from comment # 1. Received the following error:

###!!! Deadlock may happen NOW!
[Parent 48117] ###!!! ASSERTION: Potential deadlock detected:
Cyclical dependency starts at
Mutex : IndirectLayerTree (currently acquired)
Cycle completed at
Mutex : IndirectLayerTree (currently acquired)
###!!! Deadlock may happen NOW!

Went through verification using the following builds:

m-c -> used changeset: 51458a066fda
* ran through the same command and didn't receive the original Deadlock

However when I tried running the command from comment #1 on aurora & esr31, I got the following error:

"The mochitest-plain command does not accept the arguments: --nested_oop"

If there's a way to test this on the other channel, please let me know! Marking m-c as verified.