Closed Bug 1349232 Opened 3 years ago Closed 3 years ago

Intermittent linux64-qr REFTEST PROCESS-CRASH | application crashed [@ mozilla::layers::CompositorVsyncScheduler::Composite]

Categories

(Core :: Graphics: WebRender, defect, P3)

55 Branch
x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 --- fixed

People

(Reporter: kats, Assigned: sotaro)

References

Details

(Keywords: intermittent-failure, Whiteboard: [gfx-noted][gecko:Composite])

Attachments

(1 file)

For example https://treeherder.mozilla.org/logviewer.html#?job_id=85344556&repo=graphics&lineNumber=6815

SIGSEGV 0x0 at https://hg.mozilla.org/projects/graphics/file/441ae3d3a5e8e5bbb05f9ca1e1fd7859e5e8b569/gfx/layers/ipc/CompositorVsyncScheduler.cpp#l240

Stack looks like:

 0  libxul.so!mozilla::layers::CompositorVsyncScheduler::Composite [CompositorVsyncScheduler.cpp:441ae3d3a5e8 : 240 + 0x7]
 1  libxul.so!mozilla::detail::RunnableMethodImpl<mozilla::layers::CompositorVsyncScheduler*, void (mozilla::layers::CompositorVsyncScheduler::*)(mozilla::TimeStamp), true, true, mozilla::TimeStamp>::Run [nsThreadUtils.h:441ae3d3a5e8 : 855 + 0x1c]
 2  libxul.so!MessageLoop::RunTask [message_loop.cc:441ae3d3a5e8 : 358 + 0x6]
 3  libxul.so!MessageLoop::DeferOrRunPendingTask [message_loop.cc:441ae3d3a5e8 : 366 + 0x5]
 4  libxul.so!MessageLoop::DoWork [message_loop.cc:441ae3d3a5e8 : 441 + 0x5]
 5  libxul.so!base::MessagePumpDefault::Run [message_pump_default.cc:441ae3d3a5e8 : 36 + 0xa]
 6  libxul.so!MessageLoop::Run [message_loop.cc:441ae3d3a5e8 : 231 + 0x8]
 7  libxul.so!base::Thread::ThreadMain [thread.cc:441ae3d3a5e8 : 179 + 0x8]
 8  libxul.so!ThreadFunc [platform_thread_posix.cc:441ae3d3a5e8 : 38 + 0x3]
 9  libpthread-2.23.so + 0x76ba
10  libc-2.23.so + 0x10682d
Whiteboard: [gfx-noted] → [gfx-noted][gecko:Composite]
It seems that there is a case that CompositorVsyncScheduler::Composite() was called since CompositorVsyncScheduler::Destroy().
Assignee: nobody → sotaro.ikeda.g
CompositorVsyncScheduler::ScheduleComposition() does not have a check if Destroy() is already called.
Attachment #8849800 - Flags: review?(mchang)
Attachment #8849800 - Flags: review?(bugmail)
Comment on attachment 8849800 [details] [diff] [review]
patch - Add destroyed check to CompositorVsyncScheduler::ScheduleComposition()

Review of attachment 8849800 [details] [diff] [review]:
-----------------------------------------------------------------

This doesn't make sense to me. I agree with your reasoning that CompositorVsyncScheduler::ScheduleComposition() was called after CompositorVsyncScheduler::Destroy(), because otherwise there should not be a composite task waiting to run with the owner being null. However, ScheduleComposition() is called from [1] and Destroy() is called from [2]. Which means that if Destroy() is called, WebRenderBridgeParent::mCompositorScheduler should be set to null [3], and so WebRenderBridgeParent::ScheduleComposition should do nothing. So I don't understand how WebRenderBridgeParent can possibly calling ScheduleComposition on the vsync scheduler *after* it calls Destroy on the vsync scheduler.

[1] http://searchfox.org/mozilla-central/rev/0079c7adf3b329bff579d3bbe6ac7ba2f6218a19/gfx/layers/wr/WebRenderBridgeParent.cpp#589
[2] http://searchfox.org/mozilla-central/rev/0079c7adf3b329bff579d3bbe6ac7ba2f6218a19/gfx/layers/wr/WebRenderBridgeParent.cpp#611
[3] http://searchfox.org/mozilla-central/rev/0079c7adf3b329bff579d3bbe6ac7ba2f6218a19/gfx/layers/wr/WebRenderBridgeParent.cpp#613
Attachment #8849800 - Flags: review?(bugmail) → review-
See Also: → 1283019
I'm not convinced that's the same issue. That one seems like a more fundamental issue with the MessageLoop, but I think the crash in this bug implies that the MessageLoop itself is working ok.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #6)
> I'm not convinced that's the same issue. That one seems like a more
> fundamental issue with the MessageLoop, but I think the crash in this bug
> implies that the MessageLoop itself is working ok.

In current implementation, WebRenderBridgeParent works independently, if root WebRenderBridgeParent is destroyed without CompositorBridgeParent::StopAndClearResources() call, non-root WebRenderBridgeParent are still active and could trigger ScheduleComposition().
Attachment #8849800 - Flags: review- → review?(bugmail)
(In reply to Sotaro Ikeda [:sotaro] from comment #7)
> (In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #6)
> > I'm not convinced that's the same issue. That one seems like a more
> > fundamental issue with the MessageLoop, but I think the crash in this bug
> > implies that the MessageLoop itself is working ok.
> 
> In current implementation, WebRenderBridgeParent works independently, if
> root WebRenderBridgeParent is destroyed without
> CompositorBridgeParent::StopAndClearResources() call, non-root
> WebRenderBridgeParent are still active and could trigger
> ScheduleComposition().

Anyway, we need to address the condition with the patch.
:kats, if you do not convince the comment 7, I am going to create another bug to address the condition.
(In reply to Mason Chang [:mchang] from comment #5)
> See https://bugzilla.mozilla.org/show_bug.cgi?id=1283019#c17

From the crash stack, I do not think bug 1283019 is related.
(In reply to Sotaro Ikeda [:sotaro] from comment #7)
> In current implementation, WebRenderBridgeParent works independently, if
> root WebRenderBridgeParent is destroyed without
> CompositorBridgeParent::StopAndClearResources() call, non-root
> WebRenderBridgeParent are still active and could trigger
> ScheduleComposition().

Ah! That makes sense, thanks. Please update the commit message to say this, as it is important to understanding why the patch works (or should work).
Comment on attachment 8849800 [details] [diff] [review]
patch - Add destroyed check to CompositorVsyncScheduler::ScheduleComposition()

Review of attachment 8849800 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with commit message updated to explain better
Attachment #8849800 - Flags: review?(mchang)
Attachment #8849800 - Flags: review?(bugmail)
Attachment #8849800 - Flags: review+
Thanks! I'll update the commit message.
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/036da7da58ea
Add destroyed check to CompositorVsyncScheduler::ScheduleComposition() r=kats
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.