Crash in [@ nsTSubstring<T>::Assign | CrashReporter::TakeMinidumpForChild]
Categories
(Toolkit :: Crash Reporting, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox67 | --- | unaffected |
firefox68 | + | fixed |
firefox69 | --- | fixed |
People
(Reporter: lizzard, Assigned: gsvelto)
References
(Regression)
Details
(Keywords: crash, regression)
Crash Data
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details |
This bug is for crash report bp-71bcfd82-94c8-4fc7-8bd1-bdbb60190521.
This signature first shows up for the initial nightly 69 build, 20190520215528.
Top 10 frames of crashing thread:
0 xul.dll nsTSubstring<char>::Assign xpcom/string/nsTSubstring.cpp:470
1 xul.dll CrashReporter::TakeMinidumpForChild toolkit/crashreporter/nsExceptionHandler.cpp:3274
2 xul.dll void mozilla::plugins::PluginModuleChromeParent::ProcessFirstMinidump dom/plugins/ipc/PluginModuleParent.cpp:1306
3 xul.dll void mozilla::plugins::PluginModuleChromeParent::ActorDestroy dom/plugins/ipc/PluginModuleParent.cpp:1381
4 xul.dll void mozilla::plugins::PPluginModuleParent::DestroySubtree ipc/ipdl/PPluginModuleParent.cpp:1485
5 xul.dll mozilla::plugins::PPluginModuleParent::OnChannelError ipc/ipdl/PPluginModuleParent.cpp:1449
6 xul.dll nsresult mozilla::detail::RunnableMethodImpl<mozilla::dom::Performance*, void xpcom/threads/nsThreadUtils.h:1174
7 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1175
8 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:486
9 xul.dll void mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:110
Comment 1•5 years ago
|
||
This code was most recently changed in bug 1547698. My guess is that pd->annotations
is null, which seems plausible given it's a UniquePtr
.
Assignee | ||
Comment 2•5 years ago
|
||
I spent some time looking at the code and I now know why it's happening (and why it wasn't caught by the tests): plugins! These are plugin crashes where we're injecting the breakpad exception handler in the plugin process. When the process crashes it will generate a minidump but not through the standard path which adds the annotations so indeed pd->annotations
is going to be null.
We haven't caught this in testing because AFAIK we have code paths that test plugin hangs but not crashes so if I it doesn't take too long I'll try to add a test covering this.,
Updated•5 years ago
|
Assignee | ||
Comment 3•5 years ago
|
||
Even better, this is actually Flash-only because it affects only the Flash plugin's own processes. For other plugins we use the regular exception handler for child processes. Yay for Flash!
Assignee | ||
Comment 4•5 years ago
|
||
Assignee | ||
Comment 5•5 years ago
|
||
With this being Flash-specific I couldn't add any tests. I'll try and see if I can test it manually by instancing a Flash plugin and crashing it externally. Somehow someway.
Comment 7•5 years ago
|
||
bugherder |
Comment 8•5 years ago
|
||
[Tracking Requested - why for this release]: This is one of the early top crashes in 68 beta.
Comment 9•5 years ago
|
||
Please request beta uplift when you get a chance.
Assignee | ||
Comment 10•5 years ago
|
||
Comment on attachment 9066687 [details]
Bug 1553226 - Don't attempt to read annotations associated with Flash process crashes
Beta/Release Uplift Approval Request
- User impact if declined: Firefox crashes when the Flash plugin crashes or hangs and is killed by the user
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): This introduces a single null check for optional data gathered by the crash reporter.
- String changes made/needed: None
Comment 11•5 years ago
|
||
Comment on attachment 9066687 [details]
Bug 1553226 - Don't attempt to read annotations associated with Flash process crashes
crash reporter fix for 68.0b5
Comment 12•5 years ago
|
||
bugherder uplift |
Updated•3 years ago
|
Description
•