Closed Bug 828969 Opened 7 years ago Closed 7 years ago

mozbrowserasyncscroll event is dispatched synchronously. Can lead to deadlocks in main process.

Categories

(Firefox OS Graveyard :: General, defect)

defect
Not set

Tracking

(blocking-basecamp:+, firefox19 wontfix, firefox20 wontfix, firefox21 fixed, b2g18 fixed)

RESOLVED FIXED
B2G C4 (2jan on)
blocking-basecamp +
Tracking Status
firefox19 --- wontfix
firefox20 --- wontfix
firefox21 --- fixed
b2g18 --- fixed

People

(Reporter: justin.lebar+bug, Assigned: justin.lebar+bug)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

In the following stack, JS listens to the mozbrowserasyncscroll event and then takes an action which results in some action in the APZC.  Since the event is fired synchronously (while holding the APZC's lock), this results in a deadlock.

This should block, because we don't want privileged apps (which can access the browser API) to be able to lock up their processes.

I think I can write a simple patch to fix this.
blocking-basecamp: --- → ?
>#0  __futex_syscall3 () at bionic/libc/arch-arm/bionic/atomics_arm.S:182
>#1  0x4009e254 in _normal_lock (mutex=0x47eeaf60) at bionic/libc/bionic/pthread.c:951
>#2  pthread_mutex_lock (mutex=0x47eeaf60) at bionic/libc/bionic/pthread.c:1041
>#3  0x40382388 in PR_Lock (lock=0x47eeaf60) at ../../../../../../ff-git2/src/nsprpub/pr/src/pthreads/ptsynch.c:174
>#4  0x41250674 in mozilla::Mutex::Lock (this=0x43df4800, aCompositionBounds=...) at ../../dist/include/mozilla/Mutex.h:74
>#5  mozilla::Monitor::Lock (this=0x43df4800, aCompositionBounds=...) at ../../dist/include/mozilla/Monitor.h:37
>#6  MonitorAutoLock (this=0x43df4800, aCompositionBounds=...) at ../../dist/include/mozilla/Monitor.h:92
>#7  mozilla::layers::AsyncPanZoomController::UpdateCompositionBounds (this=0x43df4800, aCompositionBounds=...) at ../../../../ff-git2/src/gfx/layers/ipc/AsyncPanZoomController.cpp:1229
>#8  0x40b2cb16 in mozilla::layout::RenderFrameParent::NotifyDimensionsChanged (this=0x47eeaf60, width=<value optimized out>, height=2) at ../../../../ff-git2/src/layout/ipc/RenderFrameParent.cpp:788
>#9  0x4106b6d0 in mozilla::dom::TabParent::UpdateDimensions (this=0x4714c830, rect=..., size=...) at ../../../../ff-git2/src/dom/ipc/TabParent.cpp:265
>#10 0x40be4802 in nsFrameLoader::UpdatePositionAndSize (this=0x43ae2f60, aIFrame=<value optimized out>) at ../../../../../ff-git2/src/content/base/src/nsFrameLoader.cpp:1846
>#11 0x40b1a6e0 in nsSubDocumentFrame::ReflowFinished (this=0x4a00c038) at ../../../../ff-git2/src/layout/generic/nsSubDocumentFrame.cpp:714
>#12 0x40acfd3c in PresShell::HandlePostedReflowCallbacks (this=0x48eef700, aInterruptible=false) at ../../../../ff-git2/src/layout/base/nsPresShell.cpp:3716
>#13 0x40acfd74 in PresShell::DidDoReflow (this=0x48eef700, aInterruptible=false) at ../../../../ff-git2/src/layout/base/nsPresShell.cpp:7383
>#14 0x40ad4bc0 in PresShell::ProcessReflowCommands (this=0x48eef700, aInterruptible=false) at ../../../../ff-git2/src/layout/base/nsPresShell.cpp:7681
>#15 0x40ad4ea2 in PresShell::FlushPendingNotifications (this=0x48eef700, aFlush=...) at ../../../../ff-git2/src/layout/base/nsPresShell.cpp:3906
>#16 0x40acc3ea in PresShell::FlushPendingNotifications (this=<value optimized out>, aType=<value optimized out>) at ../../../../ff-git2/src/layout/base/nsPresShell.cpp:3756
>#17 0x40bd50a4 in nsDocument::FlushPendingNotifications (this=0x46937400, aType=Flush_Layout) at ../../../../../ff-git2/src/content/base/src/nsDocument.cpp:6507
>#18 0x40be78ec in mozilla::dom::Element::GetPrimaryFrame (this=0x472d20b0, aType=Flush_Layout) at ../../../../../ff-git2/src/content/base/src/Element.cpp:1621
>#19 0x40be9688 in mozilla::dom::Element::GetStyledFrame (this=0x46937400) at ../../../../../ff-git2/src/content/base/src/Element.cpp:488
>#20 0x40be96ac in mozilla::dom::Element::GetScrollFrame (this=0x472d20b0, aStyledFrame=0xbea4cbcc) at ../../../../../ff-git2/src/content/base/src/Element.cpp:543
>#21 0x40be973a in mozilla::dom::Element::GetClientAreaRect (this=<value optimized out>) at ../../../../../ff-git2/src/content/base/src/Element.cpp:635
>#22 0x41131a3c in mozilla::dom::Element::ClientTop (cx=<value optimized out>, obj=<value optimized out>, self=0x472d20b0, vp=0x48eef700) at ../../dist/include/mozilla/dom/Element.h:671
>#23 get_clientTop (cx=<value optimized out>, obj=<value optimized out>, self=0x472d20b0, vp=0x48eef700) at ElementBinding.cpp:1475
>#24 0x41133b42 in genericGetter (cx=0x47e79aa0, argc=<value optimized out>, vp=0x42d68150) at ElementBinding.cpp:1961
>#25 0x4144cdd2 in CallJSNative (cx=0x47e79aa0, obj=<value optimized out>, fval=<value optimized out>, argc=<value optimized out>, argv=0x0, rval=0xbea4d140)
>    at ../../../../ff-git2/src/js/src/jscntxtinlines.h:373
>#26 InvokeKernel (cx=0x47e79aa0, obj=<value optimized out>, fval=<value optimized out>, argc=<value optimized out>, argv=0x0, rval=0xbea4d140) at ../../../../ff-git2/src/js/src/jsinterp.cpp:391
>#27 Invoke (cx=0x47e79aa0, obj=<value optimized out>, fval=<value optimized out>, argc=<value optimized out>, argv=0x0, rval=0xbea4d140) at ../../../../ff-git2/src/js/src/jsinterp.h:112
>#28 Invoke (cx=0x47e79aa0, obj=<value optimized out>, fval=<value optimized out>, argc=<value optimized out>, argv=0x0, rval=0xbea4d140) at ../../../../ff-git2/src/js/src/jsinterp.cpp:439
>#29 js::InvokeGetterOrSetter (cx=0x47e79aa0, obj=<value optimized out>, fval=<value optimized out>, argc=<value optimized out>, argv=0x0, rval=0xbea4d140) at ../../../../ff-git2/src/js/src/jsinterp.cpp:512
>#30 0x4146215a in js::Shape::get (cx=0x47e79aa0, obj=..., id=<value optimized out>, getHow=<value optimized out>, vp=...) at ../../../../ff-git2/src/js/src/jsscopeinlines.h:297
>#31 js_NativeGetInline (cx=0x47e79aa0, obj=..., id=<value optimized out>, getHow=<value optimized out>, vp=...) at ../../../../ff-git2/src/js/src/jsobj.cpp:3260
>#32 js_GetPropertyHelperInline (cx=0x47e79aa0, obj=..., id=<value optimized out>, getHow=<value optimized out>, vp=...) at ../../../../ff-git2/src/js/src/jsobj.cpp:3410
>#33 js::GetPropertyHelper (cx=0x47e79aa0, obj=..., id=<value optimized out>, getHow=<value optimized out>, vp=...) at ../../../../ff-git2/src/js/src/jsobj.cpp:3419
>#34 0x4144a55e in GetPropertyOperation (cx=0x47e79aa0, entryFrame=<value optimized out>, interpMode=<value optimized out>) at ../../../../ff-git2/src/js/src/jsinterpinlines.h:290
>#35 js::Interpret (cx=0x47e79aa0, entryFrame=<value optimized out>, interpMode=<value optimized out>) at ../../../../ff-git2/src/js/src/jsinterp.cpp:2235
>#36 0x4144afae in js::RunScript (cx=0x47e79aa0, script=..., fp=0x42d68040) at ../../../../ff-git2/src/js/src/jsinterp.cpp:348
>#37 0x4144b74e in InvokeKernel (cx=0x47e79aa0, args=..., construct=js::NO_CONSTRUCT) at ../../../../ff-git2/src/js/src/jsinterp.cpp:406
>#38 0x41423f70 in Invoke (cx=0x47e79aa0, argc=1121353768, vp=0x42d68010) at ../../../../ff-git2/src/js/src/jsinterp.h:112
>#39 js::CallOrConstructBoundFunction (cx=0x47e79aa0, argc=1121353768, vp=0x42d68010) at ../../../../ff-git2/src/js/src/jsfun.cpp:1091
>#40 0x4144ca90 in CallJSNative (cx=0x47e79aa0, thisv=<value optimized out>, fval=<value optimized out>, argc=<value optimized out>, argv=0xbea4d658, rval=0xbea4d798)
>    at ../../../../ff-git2/src/js/src/jscntxtinlines.h:373
>#41 InvokeKernel (cx=0x47e79aa0, thisv=<value optimized out>, fval=<value optimized out>, argc=<value optimized out>, argv=0xbea4d658, rval=0xbea4d798) at ../../../../ff-git2/src/js/src/jsinterp.cpp:391
>#42 Invoke (cx=0x47e79aa0, thisv=<value optimized out>, fval=<value optimized out>, argc=<value optimized out>, argv=0xbea4d658, rval=0xbea4d798) at ../../../../ff-git2/src/js/src/jsinterp.h:112
>#43 Invoke (cx=0x47e79aa0, thisv=<value optimized out>, fval=<value optimized out>, argc=<value optimized out>, argv=0xbea4d658, rval=0xbea4d798) at ../../../../ff-git2/src/js/src/jsinterp.cpp:439
>#44 0x413eae3a in JS_CallFunctionValue (cx=0x47e79aa0, objArg=<value optimized out>, fval=..., argc=1, argv=0xbea4d658, rval=0xbea4d798) at ../../../../ff-git2/src/js/src/jsapi.cpp:5805
>#45 0x40eedb84 in nsXPCWrappedJSClass::CallMethod (this=0x464e1d60, wrapper=<value optimized out>, methodIndex=<value optimized out>, info_=0x42b0bac8, nativeParams=0xbea4d858)
>    at ../../../../../ff-git2/src/js/xpconnect/src/XPCWrappedJSClass.cpp:1432
>#46 0x40eeb26a in nsXPCWrappedJS::CallMethod (this=0x4a46ba40, methodIndex=3, info=0x42b0bac8, params=<value optimized out>) at ../../../../../ff-git2/src/js/xpconnect/src/XPCWrappedJS.cpp:580
>#47 0x411e22f4 in PrepareAndDispatch (self=0x46949c70, methodIndex=<value optimized out>, args=0xbea4d914) at ../../../../../../../../ff-git2/src/xpcom/reflect/xptcall/src/md/unix/xptcstubs_arm.cpp:105
>#48 0x411e1a5c in SharedStub () from /Users/jlebar/code/moz/B2G/objdir-gecko/dist/bin/libxul.so
>#49 0x40c3c74a in nsEventListenerManager::HandleEventSubType (this=<value optimized out>, aListenerStruct=<value optimized out>, aListener=0x46949c70, aDOMEvent=0x471ad7c0, aCurrentTarget=0x43c173a0, 
>    aPusher=0xbea4da64) at ../../../../../ff-git2/src/content/events/src/nsEventListenerManager.cpp:922
>#50 0x40c3c88c in nsEventListenerManager::HandleEventInternal (this=0x43ae2a10, aPresContext=<value optimized out>, aEvent=0x47139100, aDOMEvent=0xbea4da54, aCurrentTarget=0x43c173a0, 
>    aEventStatus=0xbea4da58, aPusher=0xbea4da64) at ../../../../../ff-git2/src/content/events/src/nsEventListenerManager.cpp:989
>---Type <return> to continue, or q <return> to quit---
>#51 0x40c4c1c0 in nsEventListenerManager::HandleEvent (this=<value optimized out>, aVisitor=<value optimized out>, aMayHaveNewListenerManagers=<value optimized out>, aPusher=0xbea4da64)
>    at ../../../../../ff-git2/src/content/events/src/nsEventListenerManager.h:279
>#52 nsEventTargetChainItem::HandleEvent (this=<value optimized out>, aVisitor=<value optimized out>, aMayHaveNewListenerManagers=<value optimized out>, aPusher=0xbea4da64)
>    at ../../../../../ff-git2/src/content/events/src/nsEventDispatcher.cpp:183
>#53 0x40c4c2b6 in nsEventTargetChainItem::HandleEventTargetChain (this=0x4631b2a0, aVisitor=..., aCallback=0x0, aMayHaveNewListenerManagers=false, aPusher=0xbea4da64)
>    at ../../../../../ff-git2/src/content/events/src/nsEventDispatcher.cpp:313
>#54 0x40c4c936 in nsEventDispatcher::Dispatch (aTarget=<value optimized out>, aPresContext=0x472e7400, aEvent=0x47139100, aDOMEvent=<value optimized out>, aEventStatus=0xbea4daf8, aCallback=0x0, 
>    aTargets=0x0) at ../../../../../ff-git2/src/content/events/src/nsEventDispatcher.cpp:681
>#55 0x40c4cb3e in nsEventDispatcher::DispatchDOMEvent (aTarget=0x43c173a0, aEvent=0x47139100, aDOMEvent=0x471ad7c0, aPresContext=0x472e7400, aEventStatus=0xbea4daf8)
>    at ../../../../../ff-git2/src/content/events/src/nsEventDispatcher.cpp:742
>#56 0x40d9686c in DispatchCustomDOMEvent (aFrameElement=0x43c173a0, aEventName=..., aDetailValue=<value optimized out>) at ../../../../ff-git2/src/dom/browser-element/BrowserElementParent.cpp:102
>#57 0x40d96928 in mozilla::BrowserElementParent::DispatchAsyncScrollEvent (aTabParent=<value optimized out>, aContentRect=..., aContentSize=...)
>    at ../../../../ff-git2/src/dom/browser-element/BrowserElementParent.cpp:257
>#58 0x40b2d69c in mozilla::layout::RemoteContentController::SendAsyncScrollDOMEvent (this=0x43c04440, aContentRect=..., aContentSize=...) at ../../../../ff-git2/src/layout/ipc/RenderFrameParent.cpp:568
>#59 0x4124fcd8 in mozilla::layers::AsyncPanZoomController::SendAsyncScrollEvent (this=0x43df4800) at ../../../../ff-git2/src/gfx/layers/ipc/AsyncPanZoomController.cpp:1426
>#60 0x412510a0 in mozilla::layers::AsyncPanZoomController::OnTouchEnd (this=0x43df4800, aEvent=<value optimized out>) at ../../../../ff-git2/src/gfx/layers/ipc/AsyncPanZoomController.cpp:403
>#61 0x412511c0 in mozilla::layers::AsyncPanZoomController::HandleInputEvent (this=0x43df4800, aEvent=...) at ../../../../ff-git2/src/gfx/layers/ipc/AsyncPanZoomController.cpp:277
>#62 0x412512dc in mozilla::layers::AsyncPanZoomController::ReceiveInputEvent (this=0x43df4800, aEvent=...) at ../../../../ff-git2/src/gfx/layers/ipc/AsyncPanZoomController.cpp:239
>#63 0x4125140e in mozilla::layers::AsyncPanZoomController::ReceiveInputEvent (this=0x43df4800, aEvent=..., aOutEvent=0xbea4dd10) at ../../../../ff-git2/src/gfx/layers/ipc/AsyncPanZoomController.cpp:166
>#64 0x40b2cb26 in mozilla::layout::RenderFrameParent::NotifyInputEvent (this=<value optimized out>, aEvent=..., aOutEvent=0x1) at ../../../../ff-git2/src/layout/ipc/RenderFrameParent.cpp:779
>#65 0x4106b140 in mozilla::dom::TabParent::MaybeForwardEventToRenderFrame (this=<value optimized out>, aEvent=..., aOutEvent=0xbea4dd10) at ../../../../ff-git2/src/dom/ipc/TabParent.cpp:1244
>#66 0x4106babc in mozilla::dom::TabParent::SendRealTouchEvent (this=0x4714c830, event=...) at ../../../../ff-git2/src/dom/ipc/TabParent.cpp:449
>#67 0x40c412aa in nsEventStateManager::DispatchCrossProcessEvent (this=<value optimized out>, aEvent=0x471ad7c0, aFrameLoader=<value optimized out>, aStatus=0xbea4e0cc)
>    at ../../../../../ff-git2/src/content/events/src/nsEventStateManager.cpp:1549
>#68 0x40c43e4c in nsEventStateManager::HandleCrossProcessEvent (this=<value optimized out>, aEvent=0xbea4e5b0, aTargetFrame=<value optimized out>, aStatus=0xbea4e0cc)
>    at ../../../../../ff-git2/src/content/events/src/nsEventStateManager.cpp:1733
>#69 0x40c43ed4 in nsEventStateManager::PostHandleEvent (this=0x46cd2020, aPresContext=0x472e7400, aEvent=0xbea4e5b0, aTargetFrame=0x4398b298, aStatus=0xbea4e0cc)
>    at ../../../../../ff-git2/src/content/events/src/nsEventStateManager.cpp:3118
>#70 0x40ad39ba in PresShell::HandleEventInternal (this=0x48eef700, aEvent=0xbea4e5b0, aStatus=<value optimized out>) at ../../../../ff-git2/src/layout/base/nsPresShell.cpp:6582
>#71 0x40ad3b48 in PresShell::HandlePositionedEvent (this=0x48eef700, aTargetFrame=0x4398b298, aEvent=0xbea4e5b0, aEventStatus=0xbea4e0cc) at ../../../../ff-git2/src/layout/base/nsPresShell.cpp:6311
>#72 0x40ad4704 in PresShell::HandleEvent (this=0x0, aFrame=0x4398b298, aEvent=0xbea4e5b0, aDontRetargetEvents=<value optimized out>, aEventStatus=0xbea4e0cc)
>    at ../../../../ff-git2/src/layout/base/nsPresShell.cpp:6110
>#73 0x40cf067e in nsViewManager::DispatchEvent (this=<value optimized out>, aEvent=0xbea4e5b0, aView=<value optimized out>, aStatus=<value optimized out>)
>    at ../../../../ff-git2/src/view/src/nsViewManager.cpp:717
>#74 0x40cef41c in nsView::HandleEvent (this=<value optimized out>, aEvent=0xbea4e5b0, aUseAttachedEvents=<value optimized out>) at ../../../../ff-git2/src/view/src/nsView.cpp:1010
>#75 0x40fee844 in nsWindow::DispatchEvent (this=<value optimized out>, aEvent=0x1, aStatus=@0xbea4e0f4) at ../../../../ff-git2/src/widget/gonk/nsWindow.cpp:486
>#76 0x40fef0d6 in nsWindow::DispatchInputEvent (aEvent=..., aWasCaptured=0xbea4e614) at ../../../../ff-git2/src/widget/gonk/nsWindow.cpp:290
>#77 0x40fee12e in sendTouchEvent (this=<value optimized out>) at ../../../../ff-git2/src/widget/gonk/nsAppShell.cpp:197
>#78 GeckoInputDispatcher::dispatchOnce (this=<value optimized out>) at ../../../../ff-git2/src/widget/gonk/nsAppShell.cpp:445
>#79 0x40fed338 in nsAppShell::ProcessNextNativeEvent (this=0x41f45700, mayWait=<value optimized out>) at ../../../../ff-git2/src/widget/gonk/nsAppShell.cpp:727
>#80 0x4100a13a in nsBaseAppShell::DoProcessNextNativeEvent (this=0xbea4e5b0, mayWait=20, recursionDepth=0) at ../../../../ff-git2/src/widget/xpwidgets/nsBaseAppShell.cpp:139
>#81 0x4100a218 in nsBaseAppShell::OnProcessNextEvent (this=0x41f45700, thr=0x404ca880, mayWait=20, recursionDepth=0) at ../../../../ff-git2/src/widget/xpwidgets/nsBaseAppShell.cpp:298
>#82 0x411d453e in nsThread::ProcessNextEvent (this=0x404ca880, mayWait=true, result=0xbea4e7e7) at ../../../../ff-git2/src/xpcom/threads/nsThread.cpp:600
>#83 0x411b4d86 in NS_ProcessNextEvent_P (thread=0x41f45700, mayWait=true) at ../../../../ff-git2/src/xpcom/glue/nsThreadUtils.cpp:237
>#84 0x4107f34e in mozilla::ipc::MessagePump::Run (this=0x404c4400, aDelegate=0x404cb0c0) at ../../../../ff-git2/src/ipc/glue/MessagePump.cpp:117
>#85 0x411f6fd0 in MessageLoop::RunInternal (this=0x1) at ../../../../ff-git2/src/ipc/chromium/src/base/message_loop.cc:215
>#86 0x411f7086 in MessageLoop::RunHandler (this=0x404cb0c0) at ../../../../ff-git2/src/ipc/chromium/src/base/message_loop.cc:208
>#87 MessageLoop::Run (this=0x404cb0c0) at ../../../../ff-git2/src/ipc/chromium/src/base/message_loop.cc:182
>#88 0x41009d00 in nsBaseAppShell::Run (this=0x41f45700) at ../../../../ff-git2/src/widget/xpwidgets/nsBaseAppShell.cpp:163
>#89 0x40f672fc in nsAppStartup::Run (this=0x42bd0fd0) at ../../../../../ff-git2/src/toolkit/components/startup/nsAppStartup.cpp:288
>#90 0x409b0bc4 in XREMain::XRE_mainRun (this=0xbea4e984) at ../../../../ff-git2/src/toolkit/xre/nsAppRunner.cpp:3823
>#91 0x409b2c78 in XREMain::XRE_main (this=0xbea4e984, argc=<value optimized out>, argv=0xbea50b64, aAppData=<value optimized out>) at ../../../../ff-git2/src/toolkit/xre/nsAppRunner.cpp:3890
>#92 0x409b2dc4 in XRE_main (argc=1, argv=0xbea50b64, aAppData=0x20184, aFlags=<value optimized out>) at ../../../../ff-git2/src/toolkit/xre/nsAppRunner.cpp:4093
>#93 0x0000a1de in do_main (argc=1, argv=0xbea50b64) at ../../../../ff-git2/src/b2g/app/nsBrowserApp.cpp:164
>#94 main (argc=1, argv=0xbea50b64) at ../../../../ff-git2/src/b2g/app/nsBrowserApp.cpp:249
I think I have a fix for this, but I need to rebuild to test, since my device refuses to connect to any data network.
blocking-basecamp: ? → +
blocking-basecamp: + → ?
I suspect jdm didn't mean to re-nom.
blocking-basecamp: ? → +
Attached patch Patch, v1Splinter Review
Boris, I'm in a room with a bunch of DOM peers, so if you'd prefer to kick this off to one of them, that's fine.  But I figure you might not be quite as busy today/tomorrow.  :)
Attachment #700499 - Flags: review?(bzbarsky)
Comment on attachment 700499 [details] [diff] [review]
Patch, v1

Are we guaranteed that GetOwnerElement() does not return null even if the window or tab or whatever has been closed while the event is pending?

If so, r=me.  If not, please add a null-check.
Attachment #700499 - Flags: review?(bzbarsky) → review+
> Are we guaranteed that GetOwnerElement() does not return null even if the window or tab 
> or whatever has been closed while the event is pending?

No, but we have a null-check in the method we pass it to.

Thanks for the review!
https://hg.mozilla.org/integration/mozilla-inbound/rev/8fd79d4c7b17

I didn't push this to try, so we should let this cycle on inbound before pushing to b2g18.
https://hg.mozilla.org/releases/mozilla-b2g18/rev/8a39e67c802d
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → B2G C4 (2jan on)
You need to log in before you can comment on or make changes to this bug.