715767 - LayerManagerD3D10::ReportFailure is not thread-safe but gets called off the main thread

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It gets called by PlanarYCbCrImageD3D10::AllocateTextures off the main thread, but it calls gfx::LogFailure which uses nsIGfxInfo which is not thread-safe. At least, nsIGfxInfo does not use thread-safe refcounting.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Seems like this actually could be racy/crashy in release builds since GfxInfoBase::LogFailure is not thread-safe either. We'd better fix it!

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Probably the best thing to do is to make GfxInfo refcounting thread-safe, and make LogFailure thread-safe.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #2)
> Probably the best thing to do is to make GfxInfo refcounting thread-safe,
> and make LogFailure thread-safe.

Yes please.

Comment 4

[Joe Drew (not getting mail)]

This should be pretty straightforward. The first step will be to make widget/xpwidgets/GfxInfoBase.cpp use the _THREADSAFE variants of the NS_DECL_ISUPPORTS*-variant macros defined in nsISupportsImpl.h. The second step will be to make sure that there aren't any issues with any of the GfxInfo derived types; I don't *think* I expect any, but I haven't audited what usage of GfxInfoBase the derived types use nowadays.

Finally, you'll need to add a Mutex member to GfxInfoBase, and lock it on entrance to GfxInfoBase::LogFailure().

Comment 5

[Joe Kidd]

Hi

I'd be extremely interested in working on it. Obviously if I can count on your help, when necessary.

BR

Comment 6

[Andrew Quartey [:drexler]]

Attachment: patch

Comment 7

[Joe Drew (not getting mail)]

Comment on attachment 715224 [details] [diff] [review]
patch

Review of attachment 715224 [details] [diff] [review]:
-----------------------------------------------------------------

Lovely.

Comment 8

[Joe Drew (not getting mail)]

Andrew, do you have try access? If so, can you push this to try?

Comment 9

[Andrew Quartey [:drexler]]

(In reply to Joe Drew (:JOEDREW! \o/) from comment #8)
> Andrew, do you have try access? If so, can you push this to try?

Yes, i did before posting the patch: https://tbpl.mozilla.org/?tree=Try&rev=dd4be570b902.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0470103ac4a8

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0470103ac4a8