Crash [@ mozilla::widget::WindowHook::Lookup] on shutdown with MaxTo enabled

RESOLVED FIXED in mozilla10

Status

()

Core
Widget: Win32
--
critical
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: bbondy)

Tracking

({crash, relnote})

Trunk
mozilla10
x86
Windows 7
crash, relnote
Points:
---

Firefox Tracking Flags

(blocking2.0 -)

Details

(crash signature, URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

7 years ago
One of the breakpad reports on this crash mentions this:

"
I have isolated this problem of Firefox 3.6.8 crashing after it is closed to MaxTo. A piece of software designed to split the screen into a grid in order to accommodate multiple windows. Firefox does not crash when MaxTo is not running. Official web site of MaxTo is http://maxto.net/
"

And indeed, with that program running, Firefox crashes on shutdown with the following stacktrace:

http://crash-stats.mozilla.com/report/index/f8cd3bd6-86b2-452d-9942-22b042100902
0  	xul.dll  	mozilla::widget::WindowHook::Lookup  	 widget/src/windows/WindowHook.cpp:101
1 	xul.dll 	mozilla::widget::WindowHook::RemoveMonitor 	widget/src/windows/WindowHook.cpp:85
2 	xul.dll 	mozilla::widget::TaskbarTabPreview::DetachFromNSWindow 	widget/src/windows/TaskbarTabPreview.cpp:271
3 	xul.dll 	mozilla::widget::TaskbarTabPreview::~TaskbarTabPreview 	widget/src/windows/TaskbarTabPreview.cpp:73
4 	xul.dll 	mozilla::widget::TaskbarTabPreview::`vector deleting destructor' 	
5 	xul.dll 	mozilla::widget::TaskbarTabPreview::Release 	widget/src/windows/TaskbarTabPreview.cpp:53
6 	xul.dll 	DoDeferredRelease<nsISupports*> 	js/src/xpconnect/src/xpcjsruntime.cpp:489
7 	xul.dll 	XPCJSRuntime::GCCallback 	js/src/xpconnect/src/xpcjsruntime.cpp:760
8 	xul.dll 	nsCycleCollector::FinishCollection 	xpcom/base/nsCycleCollector.cpp:2641
9 	xul.dll 	DOMGCCallback 	dom/base/nsJSEnvironment.cpp:3807
10 	js3250.dll 	js_GC 	js/src/jsgc.cpp:3822
11 	xul.dll 	PL_NewDHashTable 	obj-firefox/xpcom/build/pldhash.c:223

Also crashing on trunk:
http://crash-stats.mozilla.com/report/index/bp-df344e61-ad39-48c9-b18c-70bfb2100913
(Reporter)

Updated

7 years ago
blocking2.0: --- → ?
Worth fixing, but not a blocker. Perhaps worth relnoting.
blocking2.0: ? → -
Keywords: relnote
(In reply to comment #0)
> One of the breakpad reports on this crash mentions this:
> 
> "
> I have isolated this problem of Firefox 3.6.8 crashing after it is closed to
> MaxTo. A piece of software designed to split the screen into a grid in order to
> accommodate multiple windows. Firefox does not crash when MaxTo is not running.
> Official web site of MaxTo is http://maxto.net/

I cannot reproduce this using your steps, but this should be dup of bug 556524.
(Reporter)

Comment 3

7 years ago
(In reply to comment #2)
> I cannot reproduce this using your steps, but this should be dup of bug 556524.

This one has clear steps to reproduce, bug 556524 is more a general bug with the mention that it is a topcrash. If this bug is fixed, it might fix bug 556524 too, but otoh, it might not.
Crash Signature: [@ mozilla::widget::WindowHook::Lookup]
(Assignee)

Comment 4

6 years ago
Created attachment 562209 [details] [diff] [review]
Fix for crash with taskbar preview

TaskbarPreview has a hook that will NULL out the mWnd member when WM_DESTROY is called. 
Because after it is called you can no longer remove the hook.

But if there is a bad hook in the chain, we may not be notified about WM_DESTROY in our hook.
In particular this happens if a hook before us doesn't call CallNextHookEx.

The fix is applied twice for both of the 2 subclasses of TaskbarPreview.
Assignee: nobody → netzen
Status: NEW → ASSIGNED
Attachment #562209 - Flags: review?(tellrob)
Comment on attachment 562209 [details] [diff] [review]
Fix for crash with taskbar preview

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

Seems worth a shot. Would be nice if instead of duplicating code we could use a method.
Attachment #562209 - Flags: review?(tellrob) → review+
(Assignee)

Comment 6

6 years ago
> Would be nice if instead of duplicating code we could use a method.

Sure I'll make a method in the base class and call it from both places.  

> Seems worth a shot.  

If I skip the WM_DESTROY from the hook it crashes in the same location, so I think it will fix.
(Assignee)

Comment 7

6 years ago
Created attachment 562422 [details] [diff] [review]
Fix for crash with taskbar preview. v2.

Same as before but refactored common logic into a base function
Attachment #562209 - Attachment is obsolete: true
Attachment #562422 - Flags: review?(tellrob)
(Assignee)

Comment 8

6 years ago
Created attachment 562426 [details] [diff] [review]
Fix for crash with taskbar preview. v2'

Had to rebase patch. 

Pushed to try as well:
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=801cea014212
Attachment #562422 - Attachment is obsolete: true
Attachment #562426 - Flags: review?(tellrob)
Attachment #562422 - Flags: review?(tellrob)
(Assignee)

Comment 9

6 years ago
Comment on attachment 562426 [details] [diff] [review]
Fix for crash with taskbar preview. v2'

Just going to mark this as r+ since it was previously already r+ and it passes try.
Attachment #562426 - Flags: review?(tellrob) → review+
(Assignee)

Comment 10

6 years ago
Pushed to inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/88960733383e

Comment 11

6 years ago
https://hg.mozilla.org/mozilla-central/rev/88960733383e
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
You need to log in before you can comment on or make changes to this bug.