Closed
Bug 792372
Opened 12 years ago
Closed 12 years ago
crash in mozilla::ipc::AsyncChannel::CloseWithError
Categories
(Core Graveyard :: Plug-ins, defect)
Tracking
(firefox17- verified, firefox18 verified)
VERIFIED
FIXED
mozilla18
People
(Reporter: scoobidiver, Assigned: benjamin)
Details
(Keywords: crash, regression)
Crash Data
Attachments
(1 file, 1 obsolete file)
900 bytes,
patch
|
gfritzsche
:
review+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
It's #71 top browser crasher in 17.0a2 and #116 in 18.0a1. It was a low volume crash but has happened more often since 17.0a1/20120826. The regression range might be: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f077de66e52d&tochange=b3cce81fef1a According to comments, it seems related to Flash games in Facebook. Signature mozilla::ipc::AsyncChannel::CloseWithError() More Reports Search UUID dca891fe-da47-4117-8a2d-133ce2120917 Date Processed 2012-09-17 19:56:55 Uptime 566 Last Crash 1.8 weeks before submission Install Age 9.4 minutes since version was first installed. Install Time 2012-09-17 19:47:23 Product Firefox Version 18.0a1 Build ID 20120917030530 Release Channel nightly OS Windows NT OS Version 6.1.7601 Service Pack 1 Build Architecture x86 Build Architecture Info GenuineIntel family 15 model 4 stepping 9 Crash Reason EXCEPTION_ACCESS_VIOLATION_READ Crash Address 0x0 App Notes AdapterVendorID: 0x10de, AdapterDeviceID: 0x01d1, AdapterSubsysID: 341f1458, AdapterDriverVersion: 8.15.11.8593 D3D10 Layers? D3D10 Layers- D3D9 Layers? D3D9 Layers- EMCheckCompatibility True Adapter Vendor ID 0x10de Adapter Device ID 0x01d1 Total Virtual Memory 2147352576 Available Virtual Memory 1676615680 System Memory Use Percentage 38 Available Page File 3300843520 Available Physical Memory 1323638784 Frame Module Signature Source 0 xul.dll mozilla::ipc::AsyncChannel::CloseWithError ipc/glue/AsyncChannel.cpp:799 1 xul.dll mozilla::plugins::PluginModuleParent::OnCrash dom/plugins/ipc/PluginModuleParent.cpp:1434 2 xul.dll CrashReporter::ReportInjectedCrash::Run toolkit/crashreporter/nsExceptionHandler.cpp:2311 3 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:624 4 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:82 5 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:201 6 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:175 7 xul.dll nsBaseAppShell::Run widget/xpwidgets/nsBaseAppShell.cpp:163 8 xul.dll nsAppShell::Run widget/windows/nsAppShell.cpp:232 9 xul.dll nsAppStartup::Run toolkit/components/startup/nsAppStartup.cpp:296 10 xul.dll XREMain::XRE_mainRun toolkit/xre/nsAppRunner.cpp:3834 11 xul.dll XREMain::XRE_main toolkit/xre/nsAppRunner.cpp:3911 12 xul.dll XRE_main toolkit/xre/nsAppRunner.cpp:3987 13 firefox.exe wmain toolkit/xre/nsWindowsWMain.cpp:100 14 firefox.exe __tmainCRTStartup crtexe.c:552 15 kernel32.dll BaseThreadInitThunk 16 ntdll.dll __RtlUserThreadStart 17 ntdll.dll _RtlUserThreadStart More reports at: https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Aipc%3A%3AAsyncChannel%3A%3ACloseWithError%28%29
Assignee | ||
Comment 1•12 years ago
|
||
This is a race, probably made more common by: 2bcda3ce2bc9 Chris Jones — Bug 784647: Ensure that Tasks and XPCOM events are dispatched with the same priority. r=bent A null check should suffice.
Assignee: nobody → benjamin
Assignee | ||
Comment 2•12 years ago
|
||
Attachment #662581 -
Flags: review?(georg.fritzsche)
Comment 3•12 years ago
|
||
Comment on attachment 662581 [details] [diff] [review] Nul-check, rev. 1 Review of attachment 662581 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/plugins/ipc/PluginModuleParent.cpp @@ +1431,5 @@ > void > PluginModuleParent::OnCrash(DWORD processID) > { > + ::mozilla::ipc::AsyncChannel* channel = GetIPCChannel(); > + if (channel) { Maybe i'm missing something, but PPluginModuleParent::GetIPCChannel() returns (non-overloaded) &mChannel, which can't be null. This might instead be a race with the channel being Clear()ed already?
Assignee | ||
Comment 4•12 years ago
|
||
Quite right! I believe this will work because ActorDestroy sets mShutdown correctly.
Attachment #662581 -
Attachment is obsolete: true
Attachment #662581 -
Flags: review?(georg.fritzsche)
Attachment #662996 -
Flags: review?(georg.fritzsche)
Comment 5•12 years ago
|
||
Comment on attachment 662996 [details] [diff] [review] Shutdown check, rev. 2 Review of attachment 662996 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/plugins/ipc/PluginModuleParent.cpp @@ +1431,5 @@ > void > PluginModuleParent::OnCrash(DWORD processID) > { > + if (!mShutdown) { > + GetIPCChannel->CloseWithError(); Missing the "()" after "GetIPCChannel". It might still make sense to check or assert on the channel not being null as the interface doesn't guarantee it (to avoid accidentally breaking it).
Attachment #662996 -
Flags: review?(georg.fritzsche) → review+
Assignee | ||
Comment 6•12 years ago
|
||
This one! I forgot about it but I think it's important enough to get up the channels.
tracking-firefox17:
--- → ?
tracking-firefox18:
--- → ?
Assignee | ||
Comment 7•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/970b0467c3ef
Status: NEW → RESOLVED
Closed: 12 years ago
status-firefox18:
--- → fixed
tracking-firefox18:
? → ---
Resolution: --- → FIXED
Version: 17 Branch → 18 Branch
Comment 8•12 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #6) > This one! I forgot about it but I think it's important enough to get up the > channels. This isn't a top crasher, so no need to track. That being said, we would accept a 17 uplift nomination given the low risk nature of this patch.
Assignee | ||
Comment 9•12 years ago
|
||
Comment on attachment 662996 [details] [diff] [review] Shutdown check, rev. 2 Bug caused by (feature/regressing bug #): bug 769048 User impact if declined: Firefox sometimes crashes when Flash crashes Testing completed (on m-c, etc.): landed without incident on m-c. The crash signature appears to have already been abset on m-c so I'm not sure if some other patch also made this go away on trunk. Risk to taking this patch (and alternatives if risky): It's a null check, and basically risk-free. String or UUID changes made by this patch: none
Attachment #662996 -
Flags: approval-mozilla-beta?
Attachment #662996 -
Flags: approval-mozilla-aurora?
Reporter | ||
Comment 10•12 years ago
|
||
It's already in Aurora because it landed in m-c before the last merge.
Assignee | ||
Updated•12 years ago
|
Attachment #662996 -
Flags: approval-mozilla-aurora?
Comment 11•12 years ago
|
||
Comment on attachment 662996 [details] [diff] [review] Shutdown check, rev. 2 Still early in the cycle, null check. Approved for Beta.
Attachment #662996 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee | ||
Comment 12•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/c72cdc4f3d95
Comment 14•12 years ago
|
||
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=mozilla%3A%3Aipc%3A%3AAsyncChannel%3A%3ACloseWithError%28%29 No crashes with this signature are listed in Socorro on any builds with the fix.
Updated•12 years ago
|
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•