Closed
Bug 1349232
Opened 8 years ago
Closed 8 years ago
Intermittent linux64-qr REFTEST PROCESS-CRASH | application crashed [@ mozilla::layers::CompositorVsyncScheduler::Composite]
Categories
(Core :: Graphics: WebRender, defect, P3)
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)
1.07 KB,
patch
|
kats
:
review+
|
Details | Diff | Splinter Review |
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
Reporter | ||
Updated•8 years ago
|
Whiteboard: [gfx-noted] → [gfx-noted][gecko:Composite]
Assignee | ||
Comment 1•8 years ago
|
||
It seems that there is a case that CompositorVsyncScheduler::Composite() was called since CompositorVsyncScheduler::Destroy().
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → sotaro.ikeda.g
Assignee | ||
Comment 2•8 years ago
|
||
CompositorVsyncScheduler::ScheduleComposition() does not have a check if Destroy() is already called.
Assignee | ||
Comment 3•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Attachment #8849800 -
Flags: review?(mchang)
Assignee | ||
Updated•8 years ago
|
Attachment #8849800 -
Flags: review?(bugmail)
Reporter | ||
Comment 4•8 years ago
|
||
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-
Comment 5•8 years ago
|
||
Reporter | ||
Comment 6•8 years ago
|
||
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.
Assignee | ||
Comment 7•8 years ago
|
||
(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().
Assignee | ||
Updated•8 years ago
|
Attachment #8849800 -
Flags: review- → review?(bugmail)
Assignee | ||
Comment 8•8 years ago
|
||
(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.
Assignee | ||
Comment 9•8 years ago
|
||
:kats, if you do not convince the comment 7, I am going to create another bug to address the condition.
Assignee | ||
Comment 10•8 years ago
|
||
(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.
Reporter | ||
Comment 11•8 years ago
|
||
(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).
Reporter | ||
Comment 12•8 years ago
|
||
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+
Assignee | ||
Comment 13•8 years ago
|
||
Thanks! I'll update the commit message.
Comment 14•8 years ago
|
||
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: 8 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 15•8 years ago
|
||
Target Milestone: --- → mozilla55
Updated•8 years ago
|
status-firefox53:
--- → unaffected
status-firefox54:
--- → unaffected
status-firefox-esr52:
--- → unaffected
You need to log in
before you can comment on or make changes to this bug.
Description
•