1074192 - crash in mozilla::image::DrawableFrameRef::DrawableFrameRef(mozilla::image::imgFrame*)

Status: RESOLVED FIXED

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

Description

[Aaron Train [:aaronmt]]

This bug was filed from the Socorro interface and is 
report bp-fe12f0ff-8e05-4cdb-a391-5cb102140928.
=============================================================

Comment 1

[Andrew Osmond [:aosmond] (he/him)]

This has resurfaced as a low volume crash in recent builds, related to bug 1331938.

Comment 2

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Hold imgFrame mutex when creating DrawableFrameRef, v1

When drawing an image on the main thread, imgFrame::Optimize can be called trigging a release of mRawSurface. In the meantime, a decoder thread could be calling imgFrame::DrawableFrameRef which depends on mRawSurface -- if it changes in the middle of the transaction, we are in trouble.

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1074192 - Hold the mutex in imgFrame when creating a DrawableFrameRef;

Review commit: https://reviewboard.mozilla.org/r/120626/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/120626/

Comment 4

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8847699 [details]
Bug 1074192 - Hold the mutex in imgFrame when creating a DrawableFrameRef;

https://reviewboard.mozilla.org/r/120626/#review122900

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c8ae52e3fc7
Hold the mutex in imgFrame when creating a DrawableFrameRef. r=tnikkel

Comment 6

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

https://hg.mozilla.org/mozilla-central/rev/5c8ae52e3fc7

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Please request Aurora approval on this when you get a chance.

Comment 8

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 8847699 [details]
Bug 1074192 - Hold the mutex in imgFrame when creating a DrawableFrameRef;

Approval Request Comment
[Feature/Bug causing the regression]: bug 1331938
[User impact if declined]: Low volume browser crash due to null pointer dereference.
[Is this code covered by automated tests?]: Yes, the modified code is on a frequently executed path, although the bug only occurs in particular cases for thread timing.
[Has the fix been verified in Nightly?]: Yes, to the extent it has not caused any new problems. The crash being solved is low volume.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: No.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: It is a very simple change, locking a mutex that should have been locked all along.
[String changes made/needed]: None.

Comment 9

[Gerry Chang [:gchang]]

Comment on attachment 8847699 [details]
Bug 1074192 - Hold the mutex in imgFrame when creating a DrawableFrameRef;

Fix a crash. Aurora54+.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/14f30c91790a