Closed
Bug 1416864
Opened 7 years ago
Closed 7 years ago
Crash in mozilla::gfx::DrawTargetD2D1::Flush
Categories
(Core :: Graphics, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox57 | --- | wontfix |
firefox58 | --- | fixed |
firefox59 | --- | fixed |
People
(Reporter: philipp, Assigned: dvander)
References
Details
(Keywords: crash, regression, Whiteboard: [gfx-noted])
Crash Data
Attachments
(1 file)
(deleted),
patch
|
bas.schouten
:
review+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is
report bp-8d8a83ff-16fc-4766-b28f-4fa870171113.
=============================================================
Top 9 frames of crashing thread:
0 xul.dll mozilla::gfx::DrawTargetD2D1::Flush gfx/2d/DrawTargetD2D1.cpp:175
1 xul.dll mozilla::gfx::DrawTargetD2D1::MarkChanged gfx/2d/DrawTargetD2D1.cpp:1291
2 xul.dll mozilla::gfx::DrawTargetD2D1::PrepareForDrawing gfx/2d/DrawTargetD2D1.cpp:1311
3 xul.dll mozilla::gfx::DrawTargetD2D1::DrawSurface gfx/2d/DrawTargetD2D1.cpp:187
4 xul.dll mozilla::dom::CanvasRenderingContext2D::DrawImage dom/canvas/CanvasRenderingContext2D.cpp:5339
5 xul.dll mozilla::dom::CanvasRenderingContext2D::DrawImage dom/canvas/CanvasRenderingContext2D.h:241
6 xul.dll mozilla::dom::CanvasRenderingContext2DBinding::drawImage dom/bindings/CanvasRenderingContext2DBinding.cpp:2775
7 xul.dll mozilla::dom::GenericBindingMethod dom/bindings/BindingUtils.cpp:3040
8 @0x13186e505a4
=============================================================
crashes with this signature are noticeably rising during the 58 cycle.
Comment 1•7 years ago
|
||
From the reports:
https://crash-stats.mozilla.com/signature/?signature=mozilla%3A%3Agfx%3A%3ADrawTargetD2D1%3A%3AFlush&date=%3E%3D2017-05-15T05%3A44%3A14.000Z&date=%3C2017-11-15T05%3A44%3A14.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=app_notes&_sort=-date&page=1#reports
Most of crashes are using OMTP. And the crash line is at the container's operating calls.
So, I think it might be a data racing issue.
Whiteboard: [gfx-noted]
Assignee | ||
Comment 2•7 years ago
|
||
This does seem suspicious. I wanted to add threadsafety assertions to DrawTargetD2D1, but now I think we should just copy the mutex solution from Skia.
Assignee: nobody → dvander
Status: NEW → ASSIGNED
Updated•7 years ago
|
status-firefox57:
--- → wontfix
Priority: -- → P1
Assignee | ||
Comment 3•7 years ago
|
||
SourceSurfaceD2D1 and DrawTargetD2D1 are not synchronized. I don't know if this is *the* problem, but it's hard to imagine it's not *a* problem.
Attachment #8929995 -
Flags: review?(bas)
Updated•7 years ago
|
Attachment #8929995 -
Flags: review?(bas) → review+
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e9296f9dd46
Synchronize how snapshots detach in DrawTargetD2D1. r=bas
Keywords: checkin-needed
Comment 5•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment 6•7 years ago
|
||
We still have some crashes with this signature on nightly, e.g. https://crash-stats.mozilla.com/report/index/ec1e49b3-7800-40ca-829b-3719d0171124
Should that go in a separate bug?
Flags: needinfo?(dvander)
Comment 8•7 years ago
|
||
Is this ready for a Beta uplift request?
status-firefox-esr52:
--- → unaffected
Flags: needinfo?(dvander)
Reporter | ||
Updated•7 years ago
|
Crash Signature: [@ mozilla::gfx::DrawTargetD2D1::Flush] → [@ mozilla::gfx::DrawTargetD2D1::Flush]
[@ std::_Hash<T>::equal_range | std::_Hash<T>::erase | mozilla::gfx::DrawTargetD2D1::Flush]
Assignee | ||
Comment 9•7 years ago
|
||
Comment on attachment 8929995 [details] [diff] [review]
add lock
Approval Request Comment
[Feature/Bug causing the regression]: OMTP
[User impact if declined]: Potential crash due to race condition
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]:
[Needs manual test from QE? If yes, steps to reproduce]:
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: No
[Why is the change risky/not risky?]: This just introduces a lock to synchronize access to a pointer. None of this code is re-entrant so there is no risk of deadlock.
[String changes made/needed]:
Flags: needinfo?(dvander)
Attachment #8929995 -
Flags: approval-mozilla-beta?
Comment 10•7 years ago
|
||
Comment on attachment 8929995 [details] [diff] [review]
add lock
Fix a crash. Beta58+.
Attachment #8929995 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 11•7 years ago
|
||
bugherder uplift |
Reporter | ||
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•