561871 - Hang, no hang detector, right-clicking for the silverlight context menu a lot

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Benjamin Smedberg]

If I open up notepad and place it partially over the Firefox window and then
load
http://www.microsoft.com/showcase/ja/jp/details/7b917c16-111c-425e-be85-5069a6106f2f,
I can right click on the video and then left click on notepad. If I do this
three to ten time times, Firefox becomes unresponsive with the "Silverlight"
context menu (which just says "Silverlight") rendered over it. No menus or
buttons take actions and the video is frozen. Audio continues to play in the
background from the video. 

I tried to reproduce, and eventually reproduced by right-clicking a bunch of times in silverlight. Some of those clicks dismiss the popup, and some show it. Not sure what the trigger is yet.

Comment 1

[Benjamin Smedberg]

Parent stack:

>	xul.dll!mozilla::ipc::RPCChannel::SpinInternalEventLoop()  Line 659	C++
 	xul.dll!mozilla::ipc::RPCChannel::WaitForNotify()  Line 816	C++
 	xul.dll!mozilla::ipc::RPCChannel::Call(msg=0x07bc1818, reply=0x004ce690)  Line 203	C++
 	xul.dll!mozilla::plugins::PPluginInstanceParent::CallNPP_HandleEvent(event={...}, handled=0x004ce6d0)  Line 328	C++
 	xul.dll!mozilla::plugins::PluginInstanceParent::NPP_HandleEvent(event=0x004ce784)  Line 623	C++
 	xul.dll!mozilla::plugins::PluginModuleParent::NPP_HandleEvent(instance=0x05e97d8c, event=0x004ce784)  Line 485	C++
 	xul.dll!nsNPAPIPluginInstance::HandleEvent(event=0x004ceba8, handled=0x004ce7dc)  Line 1569	C++
 	xul.dll!nsPluginInstanceOwner::ProcessEvent(anEvent={...})  Line 4608	C++
 	xul.dll!nsPluginInstanceOwner::DispatchMouseToPlugin(aMouseEvent=0x03e7d834)  Line 4102	C++
 	xul.dll!nsPluginInstanceOwner::MouseUp(aMouseEvent=0x03e7d82c)  Line 4084	C++
 	xul.dll!nsEventListenerManager::HandleEvent(aPresContext=0x08f3ec00, aEvent=0x004ceb24, aDOMEvent=0x004ce928, aCurrentTarget=0x05fc0840, aFlags=0x00000006, aEventStatus=0x004ce92c)  Line 1143	C++
 	xul.dll!nsEventTargetChainItem::HandleEventTargetChain(aVisitor={...}, aFlags=0x00000006, aCallback=0x004ce9b0, aMayHaveNewListenerManagers=0x00000001)  Line 312	C++
 	xul.dll!nsEventDispatcher::Dispatch(aTarget=, aPresContext=, aEvent=, aDOMEvent=, aEventStatus=, aCallback=, aTargets=)  Line 577	C++
 	xul.dll!PresShell::HandleEventInternal(aEvent=0x004ceb24, aView=0x05a6eb20, aStatus=0x004cea4c)  Line 6520	C++
 	xul.dll!PresShell::HandlePositionedEvent(aView=0x05a6eb20, aTargetFrame=0x00000000, aEvent=0x004ceb24, aEventStatus=0x004cea4c)  Line 6364	C++
 	xul.dll!PresShell::HandleEvent(aView=0x05a6eb20, aEvent=0x004ceb24, aEventStatus=0x004cea4c)  Line 6299	C++
 	xul.dll!nsViewManager::HandleEvent(aView=0x00000000, aPoint={...}, aEvent=0x004ceb24, aCaptured=0x004ceab4)  Line 1227	C++
 	xul.dll!nsViewManager::DispatchEvent(aEvent=0x004ceb24, aView=0x05a6eb20, aStatus=0x004ceae8)  Line 1211	C++
 	xul.dll!HandleEvent(aEvent=0x00000000)  Line 168	C++
 	xul.dll!nsWindow::DispatchEvent(event=0x004ceb24, aStatus=nsEventStatus_eIgnore)  Line 2979	C++
 	xul.dll!nsWindow::DispatchWindowEvent(event=0x00000000)  Line 3008	C++
 	xul.dll!nsWindow::DispatchMouseEvent(aEventType=0x0000012d, wParam=0x00000000, lParam=0x011002bd, aIsContextMenuKey=0x00000000, aButton=0x0005)  Line 3390	C++
 	xul.dll!ChildWindow::DispatchMouseEvent(aEventType=0x0000012d, wParam=0x00000000, lParam=0x011002bd, aIsContextMenuKey=0x00000000, aButton=0x0002)  Line 7254	C++
 	xul.dll!__SEH_epilog4_GS() 	C++
 	xul.dll!nsWindow::WindowProc(hWnd=0x00000001, msg=0x00000205, wParam=0x00000000, lParam=0x011002bd)  Line 3722	C++
 	user32.dll!_InternalCallWinProc@20() 	
 	user32.dll!_UserCallWinProcCheckWow@32() 	
 	user32.dll!_DispatchMessageWorker@8() 	
 	user32.dll!_DispatchMessageW@4() 	
 	xul.dll!mozilla::ipc::RPCChannel::SpinInternalEventLoop()  Line 661	C++
 	xul.dll!mozilla::ipc::RPCChannel::WaitForNotify()  Line 816	C++
 	xul.dll!mozilla::ipc::RPCChannel::Call(msg=0x07bc1818, reply=0x004cefa0)  Line 203	C++
 	xul.dll!mozilla::plugins::PPluginInstanceParent::CallNPP_HandleEvent(event={...}, handled=0x004cefe0)  Line 328	C++
 	xul.dll!mozilla::plugins::PluginInstanceParent::NPP_HandleEvent(event=0x004cf094)  Line 623	C++
 	xul.dll!mozilla::plugins::PluginModuleParent::NPP_HandleEvent(instance=0x05e97d8c, event=0x004cf094)  Line 485	C++
 	xul.dll!nsNPAPIPluginInstance::HandleEvent(event=0x004cf4b0, handled=0x004cf0ec)  Line 1569	C++
 	xul.dll!nsPluginInstanceOwner::ProcessEvent(anEvent={...})  Line 4608	C++
 	xul.dll!nsPluginInstanceOwner::MouseMove(aMouseEvent=0x03e7d7d4)  Line 4006	C++
 	xul.dll!nsEventListenerManager::HandleEvent(aPresContext=0x08f3ec00, aEvent=0x004cf42c, aDOMEvent=0x004cf230, aCurrentTarget=0x05fc0840, aFlags=0x00000006, aEventStatus=0x004cf234)  Line 1143	C++
 	xul.dll!nsEventTargetChainItem::HandleEventTargetChain(aVisitor={...}, aFlags=0x00000006, aCallback=0x004cf2b8, aMayHaveNewListenerManagers=0x00000001)  Line 312	C++
 	xul.dll!nsEventDispatcher::Dispatch(aTarget=, aPresContext=, aEvent=, aDOMEvent=, aEventStatus=, aCallback=, aTargets=)  Line 577	C++
 	xul.dll!nsEventStateManager::AddRef()  Line 974	C++
 	xul.dll!nsViewManager::HandleEvent(aView=0x00000000, aPoint={...}, aEvent=0x004cf42c, aCaptured=0x004cf3bc)  Line 1227	C++
 	xul.dll!nsViewManager::DispatchEvent(aEvent=0x004cf42c, aView=0x05a6eb20, aStatus=0x004cf3f0)  Line 1211	C++
 	xul.dll!HandleEvent(aEvent=0x00000000)  Line 168	C++
 	xul.dll!nsWindow::DispatchEvent(event=0x004cf42c, aStatus=nsEventStatus_eIgnore)  Line 2979	C++
 	xul.dll!nsWindow::DispatchWindowEvent(event=0x00000000)  Line 3008	C++
 	xul.dll!nsWindow::DispatchMouseEvent(aEventType=0x0000012c, wParam=0x00000002, lParam=0x011002bd, aIsContextMenuKey=0x00000000, aButton=0x0005)  Line 3390	C++
 	xul.dll!ChildWindow::DispatchMouseEvent(aEventType=0x0000012c, wParam=0x00000002, lParam=0x011002bd, aIsContextMenuKey=0x00000000, aButton=0x0000)  Line 7254	C++
 	xul.dll!nsWindow::ProcessMessage(msg=0x00000200, wParam=0x00000002, lParam=0x011002bd, aRetValue=0x004cf62c)  Line 4098	C++
 	xul.dll!nsWindow::WindowProc(hWnd=0x00000001, msg=0x00000200, wParam=0x00000002, lParam=0x011002bd)  Line 3722	C++
 	user32.dll!_InternalCallWinProc@20() 	
 	user32.dll!_UserCallWinProcCheckWow@32() 	
 	user32.dll!_DispatchMessageWorker@8() 	
 	user32.dll!_DispatchMessageW@4() 	
 	xul.dll!nsAppShell::ProcessNextNativeEvent(mayWait=0x00000001)  Line 179	C++
 	xul.dll!nsBaseAppShell::OnProcessNextEvent(thr=0x0091e880, mayWait=0x00000001, recursionDepth=0x00000000)  Line 311	C++
 	xul.dll!nsThread::ProcessNextEvent(mayWait=0x00000001, result=0x004cf838)  Line 510	C++
 	xul.dll!mozilla::ipc::MessagePump::Run(aDelegate=0x00000000)  Line 143	C++
 	xul.dll!MessageLoop::RunHandler()  Line 200	C++
 	xul.dll!MessageLoop::Run()  Line 174	C++
 	xul.dll!nsBaseAppShell::Run()  Line 180	C++
 	xul.dll!nsAppStartup::Run()  Line 184	C++
 	xul.dll!XRE_main(argc=0x00000004, argv=0x0091a8a0, aAppData=0x0091c840)  Line 3485	C++

child stack:
>	xul.dll!mozilla::ipc::RPCChannel::WaitForNotify()  Line 860	C++
 	xul.dll!mozilla::ipc::RPCChannel::Call(msg=0x007030bc, reply=0x0229eb30)  Line 203	C++
 	xul.dll!mozilla::plugins::PPluginScriptableObjectChild::CallGetParentProperty(aId=0x0071baf0, aResult=0x0229eba4, aSuccess=0x0229eb9b)  Line 882	C++
 	xul.dll!mozilla::plugins::PluginScriptableObjectChild::ScriptableGetProperty(aObject=0x0071d480, aName=0x0071baf0, aResult=0x0229ec28)  Line 263	C++
 	xul.dll!mozilla::plugins::child::_getproperty(aNPP=0x00751268, aNPObj=0x0071d480, aPropertyName=0x0071baf0, aResult=0x0229ec28)  Line 1153	C++
 	npctrl.dll!CNPBrowser::GetNPPropertyValue() 	
 	npctrl.dll!CNPBrowser::GetNPNumericPropertyValue() 	
 	npctrl.dll!CXcpNPControl::GetZoomFactor() 	
 	npctrl.dll!CommonBrowserHost::CheckForZoomChange() 	
 	npctrl.dll!CommonBrowserHost::CCheckForZoomChange::Execute() 	
 	npctrl.dll!CommonBrowserHost::ProcessExecuteMessage() 	
 	npctrl.dll!CXcpDispatcher::OnWindowMessage() 	
 	npctrl.dll!CXcpDispatcher::WindowProc() 	
 	user32.dll!_InternalCallWinProc@20() 	
 	user32.dll!_UserCallWinProcCheckWow@32() 	
 	user32.dll!_DispatchClientMessage@24() 	
 	user32.dll!___fnDWORD@4() 	
 	ntdll.dll!_KiUserCallbackDispatcher@12() 	
 	user32.dll!_NtUserTrackPopupMenuEx@24() 	
 	user32.dll!_TrackPopupMenu@28() 	
 	xul.dll!mozilla::plugins::PluginInstanceChild::TrackPopupHookProc(hMenu=0x0a6004ff, uFlags=0x00000182, x=0x000004ae, y=0x00000208, nReserved=0x00000000, hWnd=0x000b04fa, prcRect=0x00000000)  Line 964	C++
 	npctrl.dll!CXcpBrowserHost::ShowContextMenu() 	
 	npctrl.dll!CXcpNPControl::OnMouseEvent() 	
 	npctrl.dll!CXcpNPControl::HandleEvent() 	
 	npctrl.dll!CommonPluginObject::HandleEvent() 	
 	npctrl.dll!xNPP_HandleEvent() 	
 	xul.dll!mozilla::plugins::PluginInstanceChild::WinlessHandleEvent(event={...})  Line 1115	C++
 	xul.dll!mozilla::plugins::PluginInstanceChild::AnswerNPP_HandleEvent(event={...}, handled=0x0229f4dc)  Line 489	C++
 	xul.dll!mozilla::plugins::PPluginInstanceChild::OnCallReceived(msg={...}, reply=0x00000000)  Line 1202	C++
 	xul.dll!mozilla::plugins::PPluginModuleChild::OnCallReceived(msg={...}, reply=0x00000000)  Line 380	C++
 	xul.dll!mozilla::ipc::RPCChannel::DispatchIncall(call={...})  Line 484	C++
 	xul.dll!mozilla::ipc::RPCChannel::Incall(call={...}, stackDepth=0x00000000)  Line 470	C++
 	xul.dll!mozilla::ipc::RPCChannel::OnMaybeDequeueOne()  Line 411	C++
 	xul.dll!MessageLoop::RunTask(task=0x00000000)  Line 337	C++
 	xul.dll!MessageLoop::DeferOrRunPendingTask(pending_task={...})  Line 347	C++
 	xul.dll!MessageLoop::DoWork()  Line 444	C++
 	xul.dll!base::MessagePumpForUI::DoRunLoop()  Line 209	C++
 	xul.dll!base::MessagePumpWin::RunWithDispatcher(delegate=0x00000000, dispatcher=0x0229f70c)  Line 54	C++
 	xul.dll!base::MessagePumpWin::Run(delegate=0x0229f760)  Line 78	C++
 	xul.dll!MessageLoop::RunInternal()  Line 217	C++
 	xul.dll!MessageLoop::RunHandler() 	C++
 	xul.dll!MessageLoop::Run()  Line 174	C++
 	xul.dll!base::Thread::ThreadMain()  Line 168	C++

jimm, all yours man

Comment 2

[Benjamin Smedberg]

Preliminary debugging says that sModalEventCount is not being maintained correctly: I'm exiting out to the main event loop with a sModalEventCount == 1, and eventually it hits 8, and then some sequence of calls causes the spinloop to spin indefinitely and not wake up correctly.

Comment 3

[Benjamin Smedberg]

Receiving a mouseup event. We enter the nested event loop, and exit here:

http://mxr.mozilla.org/mozilla-central/source/ipc/glue/WindowsMessageLoop.cpp#659

This decrements sInnerEventLoopDepth, but doesn't decrement sModalEventCount. This exits through here:
http://mxr.mozilla.org/mozilla-central/source/ipc/glue/WindowsMessageLoop.cpp#913

Comment 4

[Jim Mathies [:jimm]]

(In reply to comment #3)
> Receiving a mouseup event. We enter the nested event loop, and exit here:
> 
> http://mxr.mozilla.org/mozilla-central/source/ipc/glue/WindowsMessageLoop.cpp#659
> 
> This decrements sInnerEventLoopDepth, but doesn't decrement sModalEventCount.
> This exits through here:
> http://mxr.mozilla.org/mozilla-central/source/ipc/glue/WindowsMessageLoop.cpp#913

That's correct. IncModalLoopCnt() is called when we receive the spin loop start event sent from the child (gOOPPSpinNativeLoopEvent, just above line 913). DecModalLoopCnt() won't get called until we receive a corresponding gOOPPStopNativeLoopEvent stop event.

I haven't been able to reproduce this out-of-balance issue with sModalEventCount.

Some possible causes:

1) Posted thread events from PluginInstanceParent are getting dropped someplace.

2) The ipc stack is not unwinding after an event is trapped in a modal loop on the child side (lost rpc in-call replies?).

3) A bug in balanced calls to EnterSpinLoop/ExitSpinLoop or IncModalLoopCnt/DecModalLoopCnt. (I tend to think this is not the case based on code inspection.)

Comment 5

[Benjamin Smedberg]

Attachment: Expose the RPCChannel from the toplevel actor, rev. 1

I know you don't like this, but I need it for the following patch so that PluginModuleParent can call spinloop methods on the RPCChannel.

Comment 6

[Benjamin Smedberg]

Attachment: Track the RPC stack explicitly using method calls, instead of Windows events, rev. 1

This isn't right yet, it's causing hangs in mochitest-ipcplugins that I haven't figured out. Basically it keeps two linked lists of sync/rpc stacks: one per-channel, and one global. The per-channel list is used to mark particular RPC frames as needing a nested event loop. The global list is used to block nested painting.

Comment 7

[Jim Mathies [:jimm]]

> -      if (!PeekMessageW(&msg, NULL, 0, 0, PM_NOREMOVE) &&
> -          !haveSentMessagesPending) {
> -        // Message was for child, we should wait a bit.
> -        SwitchToThread();
> -      }

> +    if (!haveSentMessagesPending &&
> +        !PeekMessageW(&msg, NULL, 0, 0, PM_NOREMOVE)) {
> +      // Message was for child, we should wait a bit.
> +      SwitchToThread();

Don't reverse these, PeekMessage marks input messages as old. If they aren't marked as such, they'll always trigger MsgWaitForMultipleObjects regardless of the state of mEvent, resulting in an infinite loop.

Comment 8

[Benjamin Smedberg]

Attachment: Patch A, rev. 2: Expose ProcessNativeEventsInRPCCall only

Comment 9

[Benjamin Smedberg]

Attachment: Patch B, rev. 2 - switch from messages to function calls and a linked-list stack structure

Comment 10

[Benjamin Smedberg]

Comment on attachment 441992 [details] [diff] [review]
Patch B, rev. 2 - switch from messages to function calls and a linked-list stack structure

Approval note: this bug carries some risk with it: it refactors how we do event handling and hang detection on Windows, and it's possible that it will subtly change behaviors. But I don't think we have much choice in taking it, and it's more correct than the current code. Jim and I went back and tested a bunch of the prior hang bugs and made sure there were not any obvious regressions from those.

Comment 11

[LegNeato]

Comment on attachment 441992 [details] [diff] [review]
Patch B, rev. 2 - switch from messages to function calls and a linked-list stack structure

a=LegNeato for 1.9.2.4.

Please land it on 1.9.2-default and GECKO1924_20100413_RELBRANCH.

Note this change looks scary, though initial m-c landing looks ok. If we find issues we'll back out this invasive change and go without it

Comment 12

[Benjamin Smedberg]

m-c:
http://hg.mozilla.org/mozilla-central/rev/c055ab35a22f
http://hg.mozilla.org/mozilla-central/rev/c5e9ea1e9b06

Comment 13

[Benjamin Smedberg]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/c4a50407b2b8
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/97e487ed72d8

and relbranch:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/bd13e1649691
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/c7b9abca5bb2

Comment 14

[Al Billings [:abillings - ex-MoCo]]

Verified in 1.9.2 as fixed now.