Last Comment Bug 551242 - Hang (caught by hang detector) with flash and alt key
: Hang (caught by hang detector) with flash and alt key
Status: RESOLVED FIXED
[fixed-lorentz]
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: x86 Windows 7
: -- normal (vote)
: ---
Assigned To: Jim Mathies [:jimm]
:
Mentors:
Depends on:
Blocks: OOPP LorentzBeta1
  Show dependency treegraph
 
Reported: 2010-03-09 12:24 PST by Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg]
Modified: 2010-04-07 06:10 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.4-fixed


Attachments
fix (5.64 KB, patch)
2010-03-10 12:54 PST, Jim Mathies [:jimm]
no flags Details | Diff | Splinter Review
fix (2.84 KB, patch)
2010-03-10 12:58 PST, Jim Mathies [:jimm]
no flags Details | Diff | Splinter Review
fix (3.10 KB, patch)
2010-03-10 14:11 PST, Jim Mathies [:jimm]
bent.mozilla: review+
Details | Diff | Splinter Review

Description Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2010-03-09 12:24:48 PST
Hang, caught by the hang detector, with flash context menu and the alt key:

STR:

* load a youtube video, e.g. http://www.youtube.com/watch?v=d1X4r2nHSb0&feature=rec-rn-2r-3-HM
* right-click the video to show the flash context menu
* hit the alt key a few times

In release builds the hang detector kicks in and kills off the plugin process after 10 seconds. This may be related to the topcrash bug 536666 / bug 546035, I first tried these steps because of a comment in one of those crash reports.
Comment 1 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2010-03-09 12:27:27 PST
I have this in recording, haven't diagnosed yet.
Comment 2 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2010-03-09 14:12:13 PST
Child is processing Windows messages, ends up at:

>	_DefWindowProcA@16()	
 	6c1a88a1	
 	[Frames below may be incorrect and/or missing, no symbols loaded for npswf32.dll]	
 	_NtUserPostThreadMessage@16()	
 	OnShellHookLanguage()	
 	_ShellProc()	
 	000d02a0	
 	_InternalCallWinProc@20()	
 	_UserCallWinProcCheckWow@32()	
 	ffff027f	
 	_InternalCallWinProc@20()	
 	_UserCallWinProcCheckWow@32()	
 	_CallWindowProcAorW@24()	
 	_CallWindowProcW@20()	
 	mozilla::plugins::PluginInstanceChild::PluginWindowProc(hWnd=0x000d02a0, message=0x00000105, wParam=0x00000012, lParam=0xc0380001)	C++

message 0x105 is WM_SYSKEYUP

Parent is sending an unrelated RPC message (NPP_WriteReady). AFAICT we're not hooking whatever window receives WM_SYSKEYUP for deferred message processing, thus the deadlock.
Comment 3 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2010-03-09 14:29:52 PST
Turns out that the context menu isn't necessary, all you have to do is give the plugin focus (by tabbing to it or clicking on it) and then hit "alt" to get the hang.
Comment 4 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2010-03-09 14:35:43 PST
Jim, this is probably more important than the foxit crash.
Comment 5 Jim Mathies [:jimm] 2010-03-09 19:12:25 PST
So, flash hands wm_syskeyup to def wnd proc, which in turn generates a wm_syscommand message that gets sent to the top level window. That in current nightly builds is handed to def wnd proc as well, which generates all sorts of secondary events. With the child waiting on the return, the first ipc call from parent hangs.

I tried a couple things hoping for a quick fix - deferring wm_syscommand helped but since child is held waiting, we can still get deadlocks if ipc network is heavy. Also tried a combo replymessage call / deferred wm_syscommand, but I still managed get a hang on repeated key strokes. So there's still something going on here that's unknown, will dig into it more tomorrow morning.
Comment 6 Jim Mathies [:jimm] 2010-03-10 12:54:48 PST
Created attachment 431689 [details] [diff] [review]
fix

Handling this the same way we handle activate events did the trick. Also included is a little bit of cleanup in IPCWindowProcHandler. Also, I kept the two if statements separate because i wanted specific comments for each.
Comment 7 Jim Mathies [:jimm] 2010-03-10 12:55:34 PST
Comment on attachment 431689 [details] [diff] [review]
fix

argh, two patches combined in one.
Comment 8 Jim Mathies [:jimm] 2010-03-10 12:58:13 PST
Created attachment 431690 [details] [diff] [review]
fix
Comment 9 Jim Mathies [:jimm] 2010-03-10 14:11:36 PST
Created attachment 431711 [details] [diff] [review]
fix

some additional cleanup.
Comment 10 Jim Mathies [:jimm] 2010-03-10 14:13:17 PST
WM_SYSCOMMAND:
http://msdn.microsoft.com/en-us/library/ms646360(VS.85).aspx
Comment 11 Ben Turner (not reading bugmail, use the needinfo flag!) 2010-03-10 15:09:36 PST
Comment on attachment 431711 [details] [diff] [review]
fix

Nit, fix the indentation here, should be 2 spaces.

+      GetPropW(mWnd, L"PluginInstanceParentProperty")) {
+      ReplyMessage(0);
Comment 12 Jim Mathies [:jimm] 2010-03-10 17:29:55 PST
http://hg.mozilla.org/mozilla-central/rev/5295a7cfd05c
Comment 13 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2010-03-24 08:17:30 PDT
http://hg.mozilla.org/projects/firefox-lorentz/rev/f791b2efb911
Comment 14 Mike Beltzner [:beltzner, not reading bugmail] 2010-04-06 16:09:30 PDT
Blanket approval for Lorentz merge to mozilla-1.9.2
a=beltzner for 1.9.2.4 - please make sure to mark status1.9.2:.4-fixed
Comment 15 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2010-04-07 06:10:57 PDT
Merged into 1.9.2 at http://hg.mozilla.org/releases/mozilla-1.9.2/rev/84ba4d805430

Note You need to log in before you can comment on or make changes to this bug.