562253 - Taskbar tab preview code crashes in mozilla::widget::TaskbarTabPreview::GlobalWndProc(HWND__*, unsigned int, unsigned int, long)

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Rob Arnold [:robarnold]]

Seems to have cropped up after the fix in bug 557557.

Comment 1

[Rob Arnold [:robarnold]]

Attachment: Clear the taskbar preview pointer on destruction

The current theory is that after we destroy the taskbar preview, there are still messages waiting in the queue. If we clear the pointer on the window, we'll get NULL back from ::GetPropW so it should just fall to ::DefWindowProcW. Since no one has STR or a recorded VM session, we'll have to watch crash stats to verify this fix.

Comment 2

[Rob Arnold [:robarnold]]

Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/3a7920df7580

Comment 3

[(not currently active) Ted Mielczarek]

Do you think this is the same crash?
 	mozcrt19.dll!_purecall()  Line 54	C
>	xul.dll!mozilla::widget::TaskbarPreview::WndProc(unsigned int nMsg=28, unsigned int wParam=1, long lParam=0)  Line 290 + 0x7 bytes	C++
 	xul.dll!mozilla::widget::TaskbarTabPreview::GlobalWndProc(HWND__ * hWnd=0x003403ee, unsigned int nMsg=28, unsigned int wParam=1, long lParam=0)  Line 227 + 0x11 bytes	C++
 	user32.dll!_InternalCallWinProc@20()  + 0x23 bytes

Comment 4

[Rob Arnold [:robarnold]]

Comment on attachment 443047 [details] [diff] [review]
Clear the taskbar preview pointer on destruction

This issue does not affect 1.9.2 so long as taskbar previews are disabled (which they are by default). Still, it would be good to have this on 1.9.2 should we eventually decide to enable it (plus it reduces the crashiness for those brave souls who do have it on).

Comment 5

[Rob Arnold [:robarnold]]

Comment on attachment 443047 [details] [diff] [review]
Clear the taskbar preview pointer on destruction

Ok, this doesn't seem to have fixed the crash since there are reports from today's nightlies. New theory: we are not always destroying the window when the taskbar preview dies.

Comment 6

[Rob Arnold [:robarnold]]

Attachment: Ensure the proxy window is destroyed before the object is

Since I can't reproduce this bug, I'm still guessing at causes and hence fixes. Let's try this theory instead: when the nsWindow dies before the TaskbarTabPreview does, we set mWnd to null. Some time later, the code to disable the preview is run and SetVisible(PR_FALSE) is invoked. Seeing that mWnd is null and knowing that it's thus not safe to call TaskbarPreview::Disable or Enable, SetVisible returns without error since the calling code assuredly is not equipped to deal with the resulting exception. Some time later, the GC kicks in and decides to free the TaskbarTabPreview and does so. The proxy window is never destroyed however so a dangling reference is left hidden inside the native properties of the HWND. Some time later, a message is sent to the window, such as a request for a thumbnail or a close message. GlobalWndProc looks up the dangling ptr and attempts to invoke the virtual method assuming a valid vtable.

How's that sound for a cause? Would be nice to have reliable STR so I could check this without waiting for crash reports.

Comment 7

[Rob Arnold [:robarnold]]

Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/f666d281adf3

Going to wait to see how the crash reports look before closing. This is also responsible for some of the "empty signature" crash reports.

Comment 8

[Rob Arnold [:robarnold]]

There have been no crashes in the past 5 days vs upwards of 50 crashes/day. 
See:
http://crash-stats.mozilla.com/report/list?product=Firefox&branch=1.9.3&version=Firefox%3A3.7a5pre&platform=windows&query_search=signature&query_type=contains&query=GlobalWndProc&date=05%2F17%2F2010%2008%3A56%3A23&range_value=1&range_unit=weeks&hang_type=any&process_type=any&plugin_field=filename&plugin_query_type=exact&plugin_query=&do_query=1&signature=mozilla%3A%3Awidget%3A%3ATaskbarTabPreview%3A%3AGlobalWndProc%28HWND__*%2C%20unsigned%20int%2C%20unsigned%20int%2C%20long%29
and:
http://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A3.7a5pre&platform=windows&query_search=signature&query_type=exact&query=&date=05%2F17%2F2010%2008%3A58%3A32&range_value=1&range_unit=weeks&hang_type=any&process_type=any&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&missing_sig=EMPTY_STRING

I think this bug is fixed now.