1975979 - Feature request: untie socket name of `crashhelper` from lead process PID

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, enhancement)

Description

[Kalvin Lee]

Steps to reproduce:

Preamble: I keep Firefox Stable, Thunderbird, and Firefox Nightly all in separate PID namespaces, but have them share the same network namespace.

With b60ec46 incorporated into Nightly, I tried to launch Nightly. (I'm pointing speculatively at this CL, I didn't actually run it down with mozregression.)

Actual results:

Nightly launched, but did not load any pages. The error message

[Parent 2, IPC I/O Parent] WARNING: Failed to launch socket subprocess @Could not create an IPC channel to the crash helper (Error:0): file /builds/worker/checkouts/gecko/ipc/glue/GeckoChildProcessHost.cpp:804

filled stderr. As best I can tell, this is because the PID namespacing I use (provided by bwrap) causes the lead process to take on PID 2, and this information is given to crashhelper to bind a named socket.

When multiple applications (Stable, Nightly, and Thunderbird) use this same scheme, the first to bind the socket wins. It looks like stable will still launch (as does Thunderbird), but a recent change in Nightly means that it's no longer usable when this collision happens.

Expected results:

I would like Nightly to continue working as normal even when multiple processes in the same network namespace believe themselves to have the same PID.

Comment 1

[Kalvin Lee]

I realize this feature request applies to a fraction of a percent of users (I would not be surprised if I was the only one) and I know nothing about how crashhelper works, so it may well be that I am asking for something silly and intractable. Please feel free to close if this isn't trivial to implement.

Comment 2

[BugBot [:suhaib / :marco]]

The Bugbug bot thinks this bug should belong to the 'Core::Networking' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[Gabriele Svelto [:gsvelto]]

The issue with starting Firefox should be fixed with bug 1975853 but crash reporting won't work. This is an easy fix as the listener is only used in Windows now, we don't need it on Linux anymore.

Comment 4

[Kalvin Lee]

Sorry, I don't follow — can we not dupe this to bug 1975853? (If the listener is not required for crash reporting on Linux, can I do without it? I do like dogfooding Nightly, but having it race against distro-provided Stable for crash reporting functionality feels a bit iffy.)

Comment 5

[Gabriele Svelto [:gsvelto]]

Bug 1975853 only prevents the startup crash, but doesn't make the crash reporting functionality work. I've got a patch that I'm testing now that removes the listener entirely (as we only need it on Windows now). I'm testing it as we speak and push it out for review later today.

Comment 6

[Gabriele Svelto [:gsvelto]]

Attachment: Bug 1975979 - Remove the crash helper listener socket on Linux and macOS r=afranchuk

Comment 7

[Gabriele Svelto [:gsvelto]]

Try run is here: https://treeherder.mozilla.org/jobs?repo=try&revision=6db2d6142cf7fd57e3b91d35e9c291badfa036b0

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/cd4d03276375
https://hg.mozilla.org/integration/autoland/rev/2fbb7288bd55
Remove the crash helper listener socket on Linux and macOS r=afranchuk,geckoview-reviewers,nalexander

Comment 9

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/2fbb7288bd55

Comment 10

[Gabriele Svelto [:gsvelto]]

Attachment: Bug 1975979 - Remove the crash helper listener socket on Linux and macOS

Original Revision: https://phabricator.services.mozilla.com/D262410

Comment 11

[Phabricator Automation]

firefox-esr140 Uplift Approval Request

• User impact if declined: When launching two instances of Firefox under certain scenarios crash reporting does not work and Firefox might also not start under certain circumstances. In particular this issue was reported by enterprise users running ESR140 alongside stable/nightly
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: -
• Risk associated with taking this patch: low
• Explanation of risk level: This removes a chunk of code that was still run but not used anymore
• String changes made/needed: none
• Is Android affected?: no

Comment 12

[Phabricator Automation]

firefox-esr140 Uplift Approval Request

• User impact if declined: When launching two instances of Firefox under certain scenarios crash reporting does not work and Firefox might also not start under certain circumstances. In particular this issue was reported by enterprise users running ESR140 alongside stable/nightly
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: -
• Risk associated with taking this patch: low
• Explanation of risk level: This removes a chunk of code that was still run but not used anymore
• String changes made/needed: none
• Is Android affected?: no

Comment 13

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/d2b0dc3be147