1515375 - Crash in PLDHashTable::Search | mozilla::SandboxBroker::LaunchApp

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect, P1)

Description

[Marcia Knous [:marcia]]

[Tracking Requested - why for this release]: New crash which just surfaced in 65 but is also in nightly in small volume. We should try to figure out what the root cause may be.

This bug was filed from the Socorro interface and is
report bp-c0791583-19d8-4120-bfe5-800e70181219.
=============================================================

Seen while looking at 65 beta crash stats, present in 66 nightly as well: https://bit.ly/2LqtV1B. Windows only crash which doesn't appear to be present in previous 65 betas and doesn't affect 64. Startup crash, with almost 98% of crashes happening at startup.

Top 10 frames of crashing thread:

0 xul.dll PLDHashTable::Search xpcom/ds/PLDHashTable.cpp:497
1 xul.dll mozilla::SandboxBroker::LaunchApp security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp:238
2 xul.dll bool mozilla::ipc::GeckoChildProcessHost::PerformAsyncLaunch ipc/glue/GeckoChildProcessHost.cpp:1045
3 xul.dll bool mozilla::ipc::GeckoChildProcessHost::RunPerformAsyncLaunch ipc/glue/GeckoChildProcessHost.cpp:464
4 xul.dll nsresult mozilla::detail::RunnableMethodImpl<mozilla::ipc::GeckoChildProcessHost*, bool  xpcom/threads/nsThreadUtils.h:1158
5 xul.dll bool MessageLoop::DeferOrRunPendingTask ipc/chromium/src/base/message_loop.cc:449
6 xul.dll MessageLoop::DoWork ipc/chromium/src/base/message_loop.cc:522
7 xul.dll base::MessagePumpForIO::DoRunLoop ipc/chromium/src/base/message_pump_win.cc:421
8 xul.dll base::MessagePumpWin::Run ipc/chromium/src/base/message_pump_win.h:80
9 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:307

=============================================================

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Bug 1513101 seems like the most likely candidate.

Comment 2

[Bob Owen (:bobowen)]

I'm going to add a null check, to at least stop this crash.

Comment 3

[Bob Owen (:bobowen)]

Attachment: Null check sLaunchErrors in SandboxBroker and always accumulate if not created

Comment 4

[Bob Owen (:bobowen)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=43dfc4b1c4ab9f03eeeb36daa592715780e4a829

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7cbd48e23581
Null check sLaunchErrors in SandboxBroker and always accumulate if not created. r=handyman

Comment 6

[Bob Owen (:bobowen)]

Comment on attachment 9032753 [details] [diff] [review]
Null check sLaunchErrors in SandboxBroker and always accumulate if not created

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1395952

User impact if declined: Users who have an RDD process that fails to start, will experience a browser crash.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: No test, but should see this disappear from crash stats.

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Fairly simple null check to prevent the crash.

String changes made/needed: None

Comment 8

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/7cbd48e23581

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9032753 [details] [diff] [review]
Null check sLaunchErrors in SandboxBroker and always accumulate if not created

[Triage Comment]
Adds a null check to hopefully resolve a new topcrash on Beta. Approved for 65.0b7.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/93e2cfcc400d

Comment 11

[Bob Owen (:bobowen)]

(In reply to Bob Owen (:bobowen) from comment #6)
...
> If yes, steps to reproduce: No test, but should see this disappear from
> crash stats.

As expected this crash has disappeared in 65.0b7.