1181197 - Crashes at SharedPlanarYCbCrImage::~SharedPlanarYCbCrImage()

Status: VERIFIED FIXED

(Core :: Graphics: Layers, defect)

Description

[Steven Michaud [:smichaud] (Retired)]

https://crash-stats.mozilla.com/signature/?date=%3E%3D2015-05-01&signature=mozilla%3A%3Alayers%3A%3ASharedPlanarYCbCrImage%3A%3A%7ESharedPlanarYCbCrImage%28%29

These crashes take place on OS X and Windows (at least), here:

https://hg.mozilla.org/mozilla-central/annotate/e7e69cc8c07b/gfx/layers/ipc/SharedPlanarYCbCrImage.cpp#l40

This is in code added by bug 1144906, which landed on the trunk on 2015-06-04.  The crashes started shortly afterwards.  They have relatively low volume on Windows, but on the Mac they're a topcrasher on the 42 branch.

Comment 1

[Steven Michaud [:smichaud] (Retired)]

Attachment: Null check

These crashes are null dereferences.  mTextureClient is null at the following line:

ADDREF_MANUALLY(mTextureClient);

So the simplest way to fix them is to add a null check.  I'm not sure it's the best way, though.

Jeff, I'm asking you to review this because you're the author of the code I'm changing (of the patches for bug 1144906).  Please feel free to pass the review along if you think that's appropriate.

If my null check *isn't* the best way to fix this bug, then we'll need to find someone else to take it -- I know very little about the code I'm patching :-)

I've started a set of tryserver builds for this patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=095c43738d4a

Comment 2

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2e96e63c2db4

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2e96e63c2db4

Comment 4

[Steven Michaud [:smichaud] (Retired)]

There aren't any more of these crashes on the 42 branch since my patch landed (in builds containing my patch).

Comment 5

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8630559 [details] [diff] [review]
Null check

Approval Request Comment
[Feature/regressing bug #]: 1144906
[User impact if declined]: Mac topcrash
[Describe test coverage new/current, TreeHerder]: Baked for a week on m-c
[Risks and why]: Very low risk -- added a null check
[String/UUID change made/needed]: None

Comment 6

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8630559 [details] [diff] [review]
Null check

Seems safe and stable, let's uplift to Aurora.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e3fc822b34ce