If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Fix frequent linux64 debug crashes in 789933-1.html and re-enable the test

NEW
Unassigned

Status

()

Core
Graphics
3 years ago
a year ago

People

(Reporter: RyanVM, Unassigned)

Tracking

({leave-open})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [gfx-noted])

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
Bug 994541 was hitting frequent crashes in 789933-1.html on linux64 debug. The test was temporarily disabled to get the trees green. This bug tracks finding the root cause so it can be re-enabled.
Sample error:

https://treeherder.mozilla.org/logviewer.html#?job_id=8931507&repo=mozilla-inbound
Created attachment 8593956 [details] [diff] [review]
Don't allocate X11 textures bigger than the max xlib surface size and reenable the test

Huge canvases like the one in the failing test will fall back to BufferTextureClient. This should fix the crash.

I'll push it to try when try reopens.
Attachment #8593956 - Flags: review?(jmuizelaar)
Attachment #8593956 - Flags: review?(jmuizelaar) → review+

Comment 3

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/07e55dfe0411
Sadly, the patch doesn't solve the X11 OOM issue https://treeherder.mozilla.org/#/jobs?repo=try&revision=a202d82e13d3

We should take it anyway because it limits the risks. I landed the fix without re-enabling the test.

Updated

3 years ago
Keywords: leave-open
(Reporter)

Comment 5

3 years ago
(In reply to Nicolas Silva [:nical] from comment #4)
> We should take it anyway because it limits the risks. I landed the fix
> without re-enabling the test.

Except that you did re-enabled the test.
https://hg.mozilla.org/integration/mozilla-inbound/rev/38a2444bf818
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #5)
> (In reply to Nicolas Silva [:nical] from comment #4)
> > We should take it anyway because it limits the risks. I landed the fix
> > without re-enabling the test.
> 
> Except that you did re-enabled the test.
> https://hg.mozilla.org/integration/mozilla-inbound/rev/38a2444bf818

Gosh, indeed. Thanks!
I haven't managed to reproduce the crash locally, but if I have 5 instances running this test in a loop, the x server ends up getting killed by the low memory killer. So there is still something that tries to allocate large xlib surfaces.
https://hg.mozilla.org/mozilla-central/rev/07e55dfe0411
https://hg.mozilla.org/mozilla-central/rev/38a2444bf818
We are also attempting to create an xlib surface with the (crazy) size of the canvas with X11DataTextureSourceBasic on the compositor side. if we put a limit there and just don't render the canvas, then we'll have allocated a huge TextureClient for nothing, so if we decide to do this we might as well also not create the TextureClient and not waste memory. We could also turn X11DataTextureSourceBasic into a BigImageIterator, and break the huge allocation in smaller ones, which would not reduce the memory consumption but at least avoid the huge allocation.

At the moment a page that contains a few huge canvases can cause the x server to get taken down by the low memory killer (bringing down firefox with all of the other open applications), so my preferred solution would be to just fail to create unreasonably large canvases and not attempt to composite them.
Created attachment 8595450 [details] [diff] [review]
add a pref for the maximum canvas size and set it to the maximum xlib surface size for now
Attachment #8595450 - Flags: review?(jmuizelaar)
Attachment #8595450 - Flags: review?(jmuizelaar) → review+

Comment 11

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/37111e8319b1

Comment 12

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/15dcfcb044c8
https://hg.mozilla.org/mozilla-central/rev/37111e8319b1
https://hg.mozilla.org/mozilla-central/rev/15dcfcb044c8
Unlikely that I'll be working on this on the foreseeable future. This bug is a bit tricky to "finish" in the sense that this test as-is will always be intermittent (unless run on a machine with lots of RAM) We should not crash, now but we can't guarantee we'll be able to display super large canvases. It should probably be a crash-test instead.
Assignee: nical.bugzilla → nobody
Whiteboard: [gfx-noted]
You need to log in before you can comment on or make changes to this bug.