Linux: misleading message "Firefox was updated in the background. Please restart" when X server connection errors occur
Categories
(Core :: Widget: Gtk, defect, P3)
Tracking
()
People
(Reporter: kubuntu-user, Assigned: gerard-majax)
References
Details
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:79.0) Gecko/20100101 Firefox/79.0
Steps to reproduce:
I opened a new Firefox instance with a whole lot of other applications already open.
Actual results:
Firefox said "Firefox was updated in the background. Please restart" (but it wasn't) and offered a "restart" button. After restarting, the same happened again.
After lots of try-and-error research, it turned out that other applications wouldn't start correctly either, and when I tried to start an application from a terminal, I finally got the root cause:
xorg reported "maximum number of clients reached".
Expected results:
I would expect Firefox to report something like "X server connection failure". I'm not sure if Firefox has a chance at all to determine the exact problem. If so, it should be reported as exact as possible. If not, the text shown above would still be much better than the misleading current behaviour.
Comment 1•4 years ago
|
||
Bugbug thinks this bug should belong to this component, but please revert this change in case of error.
Updated•4 years ago
|
Comment 2•1 year ago
|
||
Does our false positive detection give us any way to avoid this, or did it already fix it?
Assignee | ||
Comment 3•1 year ago
|
||
it does not sound like the error from about:restartrequired
but something else, which our code would not catch
Assignee | ||
Comment 4•1 year ago
|
||
If this is related to xclient
exhaustion, I'm wondering if this could have been fixed by bug 1635451 directly ?
Assignee | ||
Comment 5•1 year ago
|
||
I'm not sure how we crash when exhausting x11 connections, but could we have not had the time to send build id match message over IPC? If so, then we can explicitely go into https://searchfox.org/mozilla-central/rev/b670c58fd4382ff832cc24c7e3877ded621596b2/dom/base/nsFrameLoader.cpp#3717
And if it's the case, then the current code as of today should detect we dont have a buildID mismatch and should rather show a generic crash info.
Jed, since you worked most on the semi-headless bug, do you know whether in case of a x11 connections exhaustion we would have been able to send the buildid match message?
Comment 6•1 year ago
|
||
We called gtk_init
before sending the build ID confirmation (conveniently those both happen in the same function), so a crash while connecting to the X server would have caused a restartrequired false positive. Bug 1635451 means that IsHeadless()
is now true, so we skip that call to gtk_init
, so this should have stopped happening then.
Assignee | ||
Comment 7•1 year ago
|
||
OK, I verified locally. With MOZ_ENABLE_WAYLAND=0 BUILD_DEBUG=1 ./mach run --setpref security.sandbox.content.headless=false --setpref dom.ipc.avoid.gtk=false --setpref widget.non-native-theme.enabled=false 2>&1
we can run with gtk_init()
being used. Then I replaced it with a MOZ_CRASH()
as it would more or less behave if we had a X11 failure to connect, and we properly show a browser crash. Given how soon we are in the IPC step, I dont really think we can distinguish this crash enough so we can show a specific error message, but at least nowadays we do show a tab crash error instead of an about:restartrequired
.
Description
•