Closed Bug 871942 Opened 7 years ago Closed 7 years ago
crash in ns
Web Shell Window::Request Window Close
1.34 KB, patch
|Details | Diff | Splinter Review|
860 bytes, patch
|Details | Diff | Splinter Review|
It's #87 browser crasher in 21.0b7, #143 in 22.0a2, and #289 in 23.0a1. It first showed up in 21.0a1/20130109. The regression range might be (discontinuous across builds): http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dccab70d8554&tochange=7bce868864bf It might be a regression from bug 827188. Comments talk about Flash or Java. Signature nsWebShellWindow::RequestWindowClose(nsIWidget*) More Reports Search UUID edcf9f96-7cb0-414b-8335-6e8662130510 Date Processed 2013-05-10 09:37:01 Uptime 2744 Last Crash 5.7 days before submission Install Age 16.1 hours since version was first installed. Install Time 2013-05-09 17:33:11 Product Firefox Version 23.0a1 Build ID 20130509031047 Release Channel nightly OS Windows NT OS Version 6.2.9200 Build Architecture x86 Build Architecture Info GenuineIntel family 6 model 23 stepping 10 Crash Reason EXCEPTION_ACCESS_VIOLATION_READ Crash Address 0x8 App Notes AdapterVendorID: 0x1002, AdapterDeviceID: 0x9553, AdapterSubsysID: 00000000, AdapterDriverVersion: 8.970.100.7000 D2D? D2D+ DWrite? DWrite+ D3D10 Layers? D3D10 Layers+ D3D10 Layers- D3D9 Layers? D3D9 Layers+ Processor Notes sp-processor07_phx1_mozilla_com_16448:2012; MDSW emitted too many frames, triggering truncation EMCheckCompatibility True Adapter Vendor ID 0x1002 Adapter Device ID 0x9553 Total Virtual Memory 2147352576 Available Virtual Memory 1159766016 System Memory Use Percentage 68 Available Page File 2557849600 Available Physical Memory 1008316416 Frame Module Signature Source 0 xul.dll nsWebShellWindow::RequestWindowClose xpfe/appshell/src/nsWebShellWindow.cpp:302 1 xul.dll nsWindow::ProcessMessage widget/windows/nsWindow.cpp:4766 2 xul.dll nsWindow::WindowProcInternal widget/windows/nsWindow.cpp:4388 3 xul.dll CallWindowProcCrashProtected xpcom/base/nsCrashOnException.cpp:32 4 xul.dll nsWindow::WindowProc widget/windows/nsWindow.cpp:4340 5 user32.dll InternalCallWinProc 6 user32.dll UserCallWinProcCheckWow 7 user32.dll DispatchClientMessage 8 ntdll.dll KiUserCallbackDispatcher 9 xul.dll xul.dll@0xb5630 10 user32.dll RealDefWindowProcW 11 uxtheme.dll uxtheme.dll@0x2d710 12 uxtheme.dll uxtheme.dll@0x2e27 13 uxtheme.dll uxtheme.dll@0x2a39 14 user32.dll GetRealWindowOwner 15 user32.dll InternalCallWinProc 16 user32.dll UserCallWinProcCheckWow 17 user32.dll CallWindowProcW 18 xul.dll nsWindow::WindowProcInternal widget/windows/nsWindow.cpp:4393 19 xul.dll CallWindowProcCrashProtected xpcom/base/nsCrashOnException.cpp:32 20 xul.dll nsWindow::WindowProc widget/windows/nsWindow.cpp:4340 21 user32.dll InternalCallWinProc 22 user32.dll UserCallWinProcCheckWow 23 user32.dll CallWindowProcW 24 xul.dll mozilla::ipc::windows::DeferredSendMessage::Run ipc/glue/WindowsMessageLoop.cpp:987 25 xul.dll `anonymous namespace'::DeferredMessageHook ipc/glue/WindowsMessageLoop.cpp:132 26 user32.dll fnHkINLPCWPSTRUCTW 27 user32.dll __fnDWORD 28 ntdll.dll KiUserCallbackDispatcher 29 ntdll.dll KiUserApcDispatcher 30 xul.dll nsPluginInstanceOwner::Destroy dom/plugins/base/nsPluginInstanceOwner.cpp:2560 31 xul.dll nsObjectLoadingContent::DoStopPlugin content/base/src/nsObjectLoadingContent.cpp:2578 32 xul.dll nsObjectLoadingContent::StopPluginInstance content/base/src/nsObjectLoadingContent.cpp:2627 33 xul.dll nsObjectLoadingContent::DestroyContent content/base/src/nsObjectLoadingContent.cpp:2143 34 xul.dll mozilla::dom::HTMLObjectElement::DestroyContent content/html/content/src/HTMLObjectElement.cpp:438 35 xul.dll mozilla::dom::FragmentOrElement::DestroyContent content/base/src/FragmentOrElement.cpp:952 36 xul.dll nsDocumentViewer::Destroy layout/base/nsDocumentViewer.cpp:1618 37 xul.dll nsDocShell::Destroy docshell/base/nsDocShell.cpp:4901 38 xul.dll nsFrameLoader::Finalize content/base/src/nsFrameLoader.cpp:564 39 xul.dll nsDocument::MaybeInitializeFinalizeFrameLoaders content/base/src/nsDocument.cpp:6190 ... More reports at: https://crash-stats.mozilla.com/report/list?signature=nsWebShellWindow%3A%3ARequestWindowClose%28nsIWidget*%29
Hey look, we're re-entering. In plugin code! That's never happened before.
Assignee: nobody → jschoenick
Status: NEW → ASSIGNED
Component: Widget: Win32 → Plug-ins
I thought you would enjoy it. :)
Null-check the presShell here, and assert that the only situation in which that should occur is when the window is closing
Attachment #750803 - Flags: review?(jst)
I see no reason to stop plugins synchronously here, other than providing a vector to hit re-entrance bugs.
Attachment #750804 - Flags: review?(benjamin)
Try push because plugins: https://tbpl.mozilla.org/?tree=Try&rev=adafb5e4691d
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Comment on attachment 750803 [details] [diff] [review] Check for closing-window reentrance in nsWebShellWindow::RequestWindowClose [Approval Request Comment] Bug caused by (feature/regressing bug #): Long existing bug only recently being hit User impact if declined: Crash Testing completed (on m-c, etc.): On m-c Risk to taking this patch (and alternatives if risky): Low, missing null check. (The other patch, which touches plugins, needn't be uplifted) String or IDL/UUID changes made by this patch: None
Comment on attachment 750803 [details] [diff] [review] Check for closing-window reentrance in nsWebShellWindow::RequestWindowClose Could be a ride-along if we do a point release. #112 top crash on 21, very low risk null check.
Comment on attachment 750803 [details] [diff] [review] Check for closing-window reentrance in nsWebShellWindow::RequestWindowClose No planned FF21 dot releases, approving for Aurora/Beta given the very low risk evaluation and where we are in the cycle.
Attachment #750803 - Flags: approval-mozilla-release-
Attachment #750803 - Flags: approval-mozilla-beta?
Attachment #750803 - Flags: approval-mozilla-beta+
Attachment #750803 - Flags: approval-mozilla-aurora?
Attachment #750803 - Flags: approval-mozilla-aurora+
Should we uplift the other patch to prevent that re-entrancy from causing any other problems?
(In reply to Timothy Nikkel (:tn) from comment #13) > Should we uplift the other patch to prevent that re-entrancy from causing > any other problems? I talked with jst about this, and he believes we need to be re-entrant in this situation, so the actual bug is just the missing null-check. Given the number of plugin-related regressions we've had in the last few cycles, it's probably better to not uplift more than strictly necessary.
There are no reports in Socorro for the last 4 weeks, regarding this signature.
Comment on attachment 750804 [details] [diff] [review] Don't synchronously stop plugins in DestroyContent There is pretty strong evidence that this fixed bug 850571. Given that this has had lots of bake time now, are you confident enough in uplifting this to beta now? Or should we stick with letting it ride the trains?
Comment on attachment 750804 [details] [diff] [review] Don't synchronously stop plugins in DestroyContent [Approval Request Comment] Bug caused by (feature/regressing bug #): Unknown / Long standing User impact if declined: Per comment #16, this fixed (another) topcrash in bug 850571 Testing completed (on m-c, etc.): On m-c and now m-a without any reported regressions Risk to taking this patch (and alternatives if risky): Low, one-line patch to stop plugins asynchronously (known to be safe) in a spot where they were needlessly stopping synchronously (known to be a re-entrance hazard). String or IDL/UUID changes made by this patch: None
Attachment #750804 - Flags: approval-mozilla-beta?
There is only one Firefox 23 crash with this signature in the last 4 weeks (31st of May): https://crash-stats.mozilla.com/report/list?query_search=signature&query_type=contains&reason_type=contains&range_value=4&range_unit=weeks&hang_type=any&process_type=any&signature=nsWebShellWindow%3A%3ARequestWindowClose%28nsIWidget%2A%29 Marking the bug as verified considering that that crash doesn't have the same stack trace and it's a singular crash.
I would check that the crashes from bug 850571 get fixed, since that was the impetus for uplifting this to 23.
Comment on attachment 750804 [details] [diff] [review] Don't synchronously stop plugins in DestroyContent approving uplift, will check back to see if it does fix bug 850571 on FF23
Attachment #750804 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.