350872 - PSM can access string bundles on non-main threads

Status: RESOLVED FIXED

(Core :: Security: PSM, defect, P3)

Description

[Kai Engert (:KaiE:)]

The PSM code that brings up an error message on SSL or Certificate error messages loads the brand string bundle on a secondary thread.

We get warnings on the console when this happens, about some code not being threadsafe.

PSM should be changed to pre-load the required brand name on the main thread, to avoid loading the string bundle on the wrong thread.

Comment 1

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

I believe this was fixed by the PSM parts to bug 675221 and/or bug 703508. Now, error message strings are generated on-demand by nNSSSocketInfo's implementation of nsITransportSecurityInfo::GetErrorMessage(), and GetErrorMessage() is only called on the main thread.

Kai, Please resolve the bug as a duplicate of one of those two bugs if you agree.

Comment 2

[Kai Engert (:KaiE:)]

reassign bug owner.
mass-update-kaie-20120918

Comment 3

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Once all of the instances of this have been removed (which may have already happened), the initialization of the string bundles in nsNSSComponent::Init should be removed, because they were only being initialized in nsNSSComponent::Init because each string bundle must be accessed first on the main thread.

Comment 5

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

nsNSSCallbacks.cpp's HandshakeCallback still calls nssComponent->PIPBundleFormatStringFromName on the socket thread.

Comment 6

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

It doesn't look like HandshakeCallback calls PIPBundleFormatStringFromName any longer, but there may be other areas where this is an issue (see e.g. https://dxr.mozilla.org/mozilla-central/rev/3461f3cae78495f100a0f7d3d2e0b89292d3ec02/security/manager/ssl/TransportSecurityInfo.cpp#225 ).

Comment 7

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

All uses are on the main thread now (and most are going to be removed anyway when we remove the old certificate viewer).