Closed Bug 1331274 Opened 5 years ago Closed 5 years ago

Crash in mozilla::gfx::DrawTargetD2D1::Fill

Categories

(Core :: Graphics, defect, P2)

51 Branch
All
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox51 + fixed
firefox52 + fixed
firefox53 --- fixed

People

(Reporter: philipp, Assigned: milan)

References

Details

(Keywords: crash, regression, Whiteboard: [gfx-noted])

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-3fe1309f-a10c-4281-bccf-ec8912170115.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::gfx::DrawTargetD2D1::Fill(mozilla::gfx::Path const*, mozilla::gfx::Pattern const&, mozilla::gfx::DrawOptions const&) 	gfx/2d/DrawTargetD2D1.cpp:503
1 	xul.dll 	mozilla::dom::CanvasRenderingContext2D::Fill(mozilla::dom::CanvasWindingRule const&) 	dom/canvas/CanvasRenderingContext2D.cpp:3107
2 	xul.dll 	mozilla::dom::CanvasRenderingContext2DBinding::fill 	obj-firefox/dom/bindings/CanvasRenderingContext2DBinding.cpp:3348
3 	xul.dll 	mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) 	dom/bindings/BindingUtils.cpp:2812
4 		@0x1dfc8937 	
5 		@0x3deb45cf 	
6 		@0x181fbff0

crashes with this signature are rising on the beta channel since 51.0b12. they are affecting windows 7 & upwards and are accounting for around 0.1% of crashes on beta now: https://crash-stats.mozilla.com/signature/?release_channel=beta&signature=mozilla%3A%3Agfx%3A%3ADrawTargetD2D1%3A%3AFill&date=>%3D2016-12-15T20%3A31%3A50.000Z#graphs

this would be the changelog between 51.0b11 and beta 12: https://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=FIREFOX_51_0b11_RELEASE&tochange=FIREFOX_51_0b12_RELEASE
Priority: -- → P2
Whiteboard: [gfx-noted]
See Also: → 1318283
Hi Peter,
Can you help take a look at this one? Thanks.
Flags: needinfo?(howareyou322)
Kevin, could you take a look to see what else we can do here? I think this one is related to bug 1318283.
Flags: needinfo?(howareyou322)
Flags: needinfo?(kechen)
Assignee: nobody → kechen
Flags: needinfo?(kechen)
After a brief investigation, we get null pointer for aPath in[1], which is weird since we've checked the validation of the pointer in[2]. It might be dereferenced by other thread somewhere.

Also the crash volume increased in beta channel after firefox 51.0b12 which indicates the root cause might be the fixes between 51.0b11 and 51.0b12.


[1] https://hg.mozilla.org/releases/mozilla-beta/annotate/09142d07fd73/gfx/2d/DrawTargetD2D1.cpp#l503
[2] https://hg.mozilla.org/releases/mozilla-beta/annotate/09142d07fd73/dom/canvas/CanvasRenderingContext2D.cpp#l3097
[3] https://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=0a17d39220700e742bf37a960967480b2f8159f1&tochange=9ddd4fee07842e72ba49f1583ec5f596f6e60e72
NeedToCalculateBounds() may cause ClearTarget() to get called, which resets the path.  Bug 1318283 is sort of related, and a patch on bug 1329796 may also be necessary, but it may be enough here to check for valid aPath in the call.
Oh, and I believe this is going up because bug 1318283 patch removed an earlier crash.
If you end up modifying DrawTargetD2D1, please also make the equivalent change in DrawTargetSkia.
Comment on attachment 8827221 [details]
Bug 1331274: aPath coming into Fill method may not be valid.

https://reviewboard.mozilla.org/r/104966/#review105872
Attachment #8827221 - Flags: review?(kechen) → review+
Pushed by jacheng@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b7dfb6794455
aPath coming into Fill method may not be valid. r=kechen
Comment on attachment 8827221 [details]
Bug 1331274: aPath coming into Fill method may not be valid.

Approval Request Comment
[Feature/Bug causing the regression]: Most likely 1298552
[User impact if declined]: High volume crash in beta, aurora
[Is the change risky?]: Null pointer check, low risk
[Why is the change risky/not risky?]: The callers are fine with the early return.
Attachment #8827221 - Flags: approval-mozilla-beta?
Attachment #8827221 - Flags: approval-mozilla-aurora?
Comment on attachment 8827221 [details]
Bug 1331274: aPath coming into Fill method may not be valid.

crash fix for aurora52
Attachment #8827221 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/b7dfb6794455
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Not a high volume crash from this one signature, is it that it's showing up in others or we think it might be related to bug 1318283 ? We could still get this into 51 RC2 but it's very last minute.
Flags: needinfo?(milan)
Flags: needinfo?(kechen)
Can you also request uplift to m-r?
Comment on attachment 8827221 [details]
Bug 1331274: aPath coming into Fill method may not be valid.

Approval Request Comment
[Feature/Bug causing the regression]: Most likely 1298552
[User impact if declined]: High volume crash in beta, aurora
[Is the change risky?]: Null pointer check, low risk
[Why is the change risky/not risky?]: The callers are fine with the early return.
Attachment #8827221 - Flags: approval-mozilla-release?
Flags: needinfo?(milan)
Flags: needinfo?(kechen)
Comment on attachment 8827221 [details]
Bug 1331274: aPath coming into Fill method may not be valid.

Fix a crash. I would like to take this in 51 RC2.
Attachment #8827221 - Flags: approval-mozilla-release?
Attachment #8827221 - Flags: approval-mozilla-release+
Attachment #8827221 - Flags: approval-mozilla-beta?
Attachment #8827221 - Flags: approval-mozilla-beta+
Assignee: kechen → milan
Crash Signature: [@ mozilla::gfx::DrawTargetD2D1::Fill] → [@ mozilla::gfx::DrawTargetD2D1::Fill] [@ mozilla::gfx::DrawTargetSkia::Fill]
You need to log in before you can comment on or make changes to this bug.