Closed Bug 1553226 Opened 7 months ago Closed 7 months ago

Crash in [@ nsTSubstring<T>::Assign | CrashReporter::TakeMinidumpForChild]

Categories

(Toolkit :: Crash Reporting, defect, critical)

66 Branch
x86
Windows 10
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla69
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)

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

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.

Component: XPCOM → Crash Reporting
Flags: needinfo?(gsvelto)
Product: Core → Toolkit
Regressed by: 1547698

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.,

Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
Flags: needinfo?(gsvelto)

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!

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.

Pushed by gsvelto@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/56fea0bcb2c4
Don't attempt to read annotations associated with Flash process crashes r=froydnj
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

[Tracking Requested - why for this release]: This is one of the early top crashes in 68 beta.

Please request beta uplift when you get a chance.

Flags: needinfo?(gsvelto)

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
Flags: needinfo?(gsvelto)
Attachment #9066687 - Flags: approval-mozilla-beta?

Comment on attachment 9066687 [details]
Bug 1553226 - Don't attempt to read annotations associated with Flash process crashes

crash reporter fix for 68.0b5

Attachment #9066687 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.