Closed Bug 1726807 Opened 2 months ago Closed 2 months ago

Wayland: Some crashes with "gfx.webrender.compositor.force-enabled:true" don't trigger crash reports

Categories

(Core :: Graphics: WebRender, defect)

Firefox 91
x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
93 Branch
Tracking Status
firefox91 --- disabled
firefox92 --- fixed
firefox93 --- fixed

People

(Reporter: antheo.lerouzic, Assigned: rmader)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Firefox/91.0

Steps to reproduce:

Enable "gfx.webrender.compositor.force-enabled" on recent version of firefox, on gnome wayland at least.

Actual results:

Firefox crash very often, leaving no crash report unfortunately.

Expected results:

Firefox should not crash.

There is no crash report, but if I open firefox in a terminal, I have these lines while it crash :

Exiting due to channel error.
Exiting due to channel error.
Exiting due to channel error.
Processus arrêté

Thanks you!

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

Component: Untriaged → Graphics: WebRender
Product: Firefox → Core
OS: Unspecified → Linux
Hardware: Unspecified → x86_64
Summary: Frequent crash if "gfx.webrender.compositor.force-enabled" on true → Wayland: Frequent crash if "gfx.webrender.compositor.force-enabled" on true

I think crash reports aren't created when the Wayland compositor runs into a protocol error and closes the connection?

It might not be the same problem, but I have a repro for a Wayland compositor crash that doesn't get logged to about:crashes: open https://www.amazon.de/-/en/WER05056490001-Wera-Bit-Assortment/dp/B00I8MYMT2/257-0702117-8099114?psc=1 and click on the video (last thumbnail) under the product image.

This is an experimental feature, right? So I'm sure it should be tried on nightly, not on a stable version.

Antheon, do you see this issue on FF stable (91) or nightly (93)? As said in comment 5, it's an experimental feature, which is why we don't backport fixes to the stable channel - thus there's a high chance that the issue is fixed already.

Flags: needinfo?(antheo.lerouzic)

@rmader, I see the crash in comment 4 in Nightly (2021-08-16), and I think it's the same as this one. Do you want me to file another issue?

The issue is on stable (91) at least.
I will try on the last nightly.

Flags: needinfo?(antheo.lerouzic)

(In reply to Laurențiu Nicola from comment #7)

@rmader, I see the crash in comment 4 in Nightly (2021-08-16), and I think it's the same as this one. Do you want me to file another issue?

The issue here is still quite unspecific (crash without crash report) - lets continue here then. Could you try to reproduce that crash when running FF with WAYLAND_DEBUG=1? Thanks!

Attached file compositor crash
```

It seems to work with nightly, thanks!

Still crashes for me on the latest Nightly.

(In reply to antheo.lerouzic from comment #11)

It seems to work with nightly, thanks!

Thanks for testing!

(In reply to Laurențiu Nicola from comment #10)

So we have

[1496219.901] wl_display@1.error(wp_viewport@342, 0, "x and y values must be zero or positive and width and height valuest must be positive or all values must be -1 to unset the vie")
Gdk-Message: 17:10:37.995: Error 71 (Protocol error) dispatching to Wayland display.

which gets triggered in https://searchfox.org/mozilla-central/source/gfx/layers/NativeLayerWayland.cpp#203 - and it's indeed is not obvious that we can't run into forbidden conditions. Lets take over the issue and make sure that such crashes at least trigger a proper crash report.

Summary: Wayland: Frequent crash if "gfx.webrender.compositor.force-enabled" on true → Wayland: Some crashes with "gfx.webrender.compositor.force-enabled:true" don't trigger crash reports

Submitting invalid values to the Wayland compositor will trigger a
protocol error, which in turn will trigger an unhandled crash.
In order to create crash reports in such cases, assert on valid
values in such cases.

In the long run we can replace this with a general Wayland protocol
error handler.

Assignee: nobody → robert.mader
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Do we have a bug on the fact that the crash reporter isn't catching this crash? It seems worth filing one.

Laurențiu: I can reproduce the issue and it looks like a rounding issue. The values I get are: 0,000000 -0,015625 854,000000 480,000000 - just enough to trigger the protocol error :) Thanks for spotting and finding a reproducer!

See Also: → 1726923
Keywords: leave-open

(In reply to Markus Stange [:mstange] from comment #15)

Do we have a bug on the fact that the crash reporter isn't catching this crash? It seems worth filing one.

Right, filed bug 1726923. The last time I checked there was still something missing from the libwayland side to make this straight forward, but that may have changed by now.

Pushed by robert.mader@posteo.de:
https://hg.mozilla.org/integration/autoland/rev/7d4a2898ecf1
Assert on valid wp_viewport values, r=gfx-reviewers,mstange

We always expect integer values here, even with fractional scaling,
and if floating point errors cause bufferClip.x or bufferClip.y to
fall slightly below 0, this will cause a protocol error.
We can savely avoid this by simply rounding the rect.

Pushed by robert.mader@posteo.de:
https://hg.mozilla.org/integration/autoland/rev/23f3a0afb545
Avoid crashes caused by floating point errors, r=gfx-reviewers,lsalzman
Keywords: leave-open
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 93 Branch

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Blocks: 1726954

@rmader: my crash seems fixed now, thanks!

Comment on attachment 9237354 [details]
Bug 1726807 - Assert on valid wp_viewport values, r=#gfx-reviewers

Beta/Release Uplift Approval Request

  • User impact if declined: Some crashes do not create crash reports (first patch).
    Crashes because of floating point errors (second patch).
    Both happens only with the experimental feature gfx.webrender.compositor.force-enabled set to true.
  • 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): Both patches are rather trivial and the code paths are exclusively used with the experimental setting enabled.
  • String changes made/needed:
Attachment #9237354 - Flags: approval-mozilla-beta?
Attachment #9237367 - Flags: approval-mozilla-beta?

(In reply to Laurențiu Nicola from comment #24)

@rmader: my crash seems fixed now, thanks!

Thanks for helping discovering and fixing this bug!

Duplicate of this bug: 1727198

Comment on attachment 9237354 [details]
Bug 1726807 - Assert on valid wp_viewport values, r=#gfx-reviewers

Approved for 92.0b8.

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