Starting Fennec on a restricted wireless network currently brings up an SSL cert error dialog which triggers this assertion: #0 RealBreak () at /home/t_mattjo/src/firefox/mobilebase/xpcom/base/nsDebugImpl.cpp:407 #1 0x0258d684 in Break (aMsg= 0xb00fe48c "###!!! ASSERTION: nsPermissionManager not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file /home/t_mattjo/src/firefox/mobilebase/extensions/cookie/nsPermissionManager.cpp, line"...) at /home/t_mattjo/src/firefox/mobilebase/xpcom/base/nsDebugImpl.cpp:503 #2 0x0258d649 in NS_DebugBreak_P (aSeverity=1, aStr=0x2f5bb98 "nsPermissionManager not thread-safe", aExpr=0x2f5bb64 "_mOwningThread.GetThread() == PR_GetCurrentThread()", aFile= 0x2f5ba7c "/home/t_mattjo/src/firefox/mobilebase/extensions/cookie/nsPermissionManager.cpp", aLine=171) at /home/t_mattjo/src/firefox/mobilebase/xpcom/base/nsDebugImpl.cpp:371 #3 0x0209e29b in nsPermissionManager::AddRef (this=0xb31b0be0) at /home/t_mattjo/src/firefox/mobilebase/extensions/cookie/nsPermissionManager.cpp:171 #4 0x019c1e77 in nsCOMPtr<nsISupports>::nsCOMPtr (this=0xb00fe904, aSmartPtr=...) at ../../dist/include/nsCOMPtr.h:906 #5 0x02572e11 in nsComponentManagerImpl::GetServiceByContractID (this=0xb7dc3220, aContractID=0x2f33a94 "@mozilla.org/permissionmanager;1", aIID=..., result=0xb00fe9fc) at /home/t_mattjo/src/firefox/mobilebase/xpcom/components/nsComponentManager.cpp:1613 #6 0x025063a2 in CallGetService (aContractID=0x2f33a94 "@mozilla.org/permissionmanager;1", aIID=..., aResult=0xb00fe9fc) at nsComponentManagerUtils.cpp:94 #7 0x025068d6 in nsGetServiceByContractIDWithError::operator() (this=0xb00fea60, aIID=..., aInstancePtr=0xb00fe9fc) at nsComponentManagerUtils.cpp:288 #8 0x01fc4f52 in nsCOMPtr<nsIPermissionManager>::assign_from_gs_contractid_with_error (this=0xae17369c, gs=..., aIID=...) at ../../../../dist/include/nsCOMPtr.h:1262 #9 0x01fc4e30 in nsCOMPtr<nsIPermissionManager>::operator= (this=0xae17369c, rhs=...) at ../../../../dist/include/nsCOMPtr.h:721 #10 0x01fc3a9a in nsStrictTransportSecurityService::Init (this=0xae173690) at /home/t_mattjo/src/firefox/mobilebase/security/manager/boot/src/nsStrictTransportSecurityService.cpp:80 #11 0x01fc212a in nsStrictTransportSecurityServiceConstructor (aOuter=0x0, aIID=..., aResult=0xb00feb38) at /home/t_mattjo/src/firefox/mobilebase/security/manager/boot/src/nsBOOTModule.cpp:49 #12 0x02514d28 in mozilla::GenericFactory::CreateInstance (this=0xae173680, aOuter=0x0, aIID=..., aResult=0xb00feb38) at GenericFactory.cpp:48 #13 0x0257244f in nsComponentManagerImpl::CreateInstanceByContractID (this=0xb7dc3220, aContractID=0x2f3c3a9 "@mozilla.org/stsservice;1", aDelegate=0x0, aIID=..., aResult=0xb00feb38) at /home/t_mattjo/src/firefox/mobilebase/xpcom/components/nsComponentManager.cpp:1315 #14 0x025730c2 in nsComponentManagerImpl::GetServiceByContractID (this=0xb7dc3220, aContractID=0x2f3c3a9 "@mozilla.org/stsservice;1", aIID=..., result=0xb00fec2c) at /home/t_mattjo/src/firefox/mobilebase/xpcom/components/nsComponentManager.cpp:1676 #15 0x025063a2 in CallGetService (aContractID=0x2f3c3a9 "@mozilla.org/stsservice;1", aIID=..., aResult=0xb00fec2c) at nsComponentManagerUtils.cpp:94 #16 0x02506890 in nsGetServiceByContractID::operator() (this=0xb00fec44, aIID=..., aInstancePtr=0xb00fec2c) at nsComponentManagerUtils.cpp:278 #17 0x0110cad4 in nsCOMPtr<nsIStrictTransportSecurityService>::assign_from_gs_contractid (this=0xb00fed94, gs=..., aIID=...) at ../../../dist/include/nsCOMPtr.h:1252 #18 0x01feb704 in nsCOMPtr<nsIStrictTransportSecurityService>::nsCOMPtr (this=0xb00fed94, gs=...) at ../../../../dist/include/nsCOMPtr.h:627 #19 0x01fe9c25 in nsNSSBadCertHandler (arg=0xb058f710, sslSocket=0xae1bf0e0) at /home/t_mattjo/src/firefox/mobilebase/security/manager/ssl/src/nsNSSIOLayer.cpp:3499 #20 0x0036aaa6 in ssl3_HandleCertificate (ss=0xae162000, b= 0xae1a0388 "\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245"..., length=0) at ssl3con.c:7911 #21 0x0036c49a in ssl3_HandleHandshakeMessage (ss=0xae162000, b=0xae1a0004 "", length=900) at ssl3con.c:8603 #22 0x0036c928 in ssl3_HandleHandshake (ss=0xae162000, origBuf=0xae162250) at ssl3con.c:8727 #23 0x0036d40a in ssl3_HandleRecord (ss=0xae162000, cText=0xb00ff08c, databuf=0xae162250) at ssl3con.c:9066 #24 0x0036e527 in ssl3_GatherCompleteHandshake (ss=0xae162000, flags=0) at ssl3gthr.c:209 #25 0x0037117b in ssl_GatherRecord1stHandshake (ss=0xae162000) at sslcon.c:1258 #26 0x0037c824 in ssl_Do1stHandshake (ss=0xae162000) at sslsecur.c:151 #27 0x0037ea68 in ssl_SecureSend (ss=0xae162000, buf= 0xae110000 "GET /en-US/mobile/api/1.5/list/featured/all/4/Linux/4.0b3pre HTTP/1.1\r\nHost: services.addons.mozilla.org\r\nUser-Agent: Mozilla/5.0 (X11; Linux i686; rv:2.0b8pre) Gecko/20101213 Firefox/4.0b8pre Fennec/"..., len=631, flags=0) at sslsecur.c:1213 #28 0x0037ec22 in ssl_SecureWrite (ss=0xae162000, buf= 0xae110000 "GET /en-US/mobile/api/1.5/list/featured/all/4/Linux/4.0b3pre HTTP/1.1\r\nHost: services.addons.mozilla.org\r\nUser-Agent: Mozilla/5.0 (X11; Linux i686; rv:2.0b8pre) Gecko/20101213 Firefox/4.0b8pre Fennec/"..., len=631) at sslsecur.c:1258 #29 0x0038628f in ssl_Write (fd=0xae1bf0e0, buf=0xae110000, len=631) at sslsock.c:1652 #30 0x01fccf3c in nsSSLThread::Run (this=0xb01705e0) at /home/t_mattjo/src/firefox/mobilebase/security/manager/ssl/src/nsSSLThread.cpp:1045 #31 0x01fcb26b in nsPSMBackgroundThread::nsThreadRunner (arg=0xb01705e0) at /home/t_mattjo/src/firefox/mobilebase/security/manager/ssl/src/nsPSMBackgroundThread.cpp:44 #32 0x00182797 in _pt_root (arg=0xb037fa80) at /home/t_mattjo/src/firefox/mobilebase/nsprpub/pr/src/pthreads/ptthread.c:187 #33 0x008ba925 in start_thread () from /lib/libpthread.so.0 #34 0x007e407e in clone () from /lib/libc.so.6 (gdb) fr 10 #10 0x01fc3a9a in nsStrictTransportSecurityService::Init (this=0xae173690) at /home/t_mattjo/src/firefox/mobilebase/security/manager/boot/src/nsStrictTransportSecurityService.cpp:80 80 mPermMgr = do_GetService(NS_PERMISSIONMANAGER_CONTRACTID, &rv); (gdb) list 75 nsresult 76 nsStrictTransportSecurityService::Init() 77 { 78 nsresult rv; 79 80 mPermMgr = do_GetService(NS_PERMISSIONMANAGER_CONTRACTID, &rv); 81 NS_ENSURE_SUCCESS(rv, rv); 82 83 return NS_OK; 84 }

Scary!

I don't think the solution here is to make the permission manager threadsafe. Looks like the real bug here is that PSM is accessing something off of the main thread that it shouldn't be.

blocking - we want to figure out what the underlying problem is.

http://mxr.mozilla.org/mozilla-central/search?string=ns_stsservice&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central shows that anyone using the STS either is on the main thread or, in the case of nsNSSBadCertHandler, creates a proxy object that ensures that any uses of the permission manager inside of the STS is main-thread only. However, if nsNSSBadCertHandler is the first user of the STS, it'll create it on the SSL thread and cause the assertion in this bug.

Assigning to Sid, who implemented STS.

(this affects Thunderbird (on linux) too) (In reply to comment #5) > Assigning to Sid, who implemented STS. Kai, did you mean to actually assign the bug to someone or add someone to the cc?

> Kai, did you mean to actually assign the bug to someone? Yes, done now, I must have missed this. Thanks for noticing. Assigning to Sid, who implemented STS.

This patch attempts to initialize nsStrictTransportSecurityService from inside nsNSSComponent::Init (which given bug 615328, should only be on the main thread). Any future calls to do_GetService(NS_STSSERVICE_CONTRACTID) should result in the already-created instance on the main thread. I also tossed a check in nsStrictTransportSecurityService::Init() to fail if the instance is Init()ed on the wrong thread. jdm: can you check to see if this works for you? Until there's a patch for bug 615328, nsStrictTransportSecurityService::Init() should fail and return an error when it's created on the wrong thread.

I don't believe I have any way of testing this, as the bug in fennec was fixed that triggered the SSL cert failure dialog.

Comment on attachment 501754 [details] [diff] [review] proposed patch I don't know if XPCOM will keep a service alive after creation. Could XPCOM release the service immediately after nsNSSComponent::Init is done? Maybe you should use a member pointer nsNSSComponent::mStrictTranspSecServ to ensure the service instance will be kept alive. This patch is correct, r=kaie but consider to update the patch and ask for r? again.

Note bug 619201 is a similar bug where the same question is being asked.

Once a service is created *successfully*, XPCOM keeps it alive until shutdown.

Thanks for your clarification, it means the patch is sufficient. However, Sid, if you believe creation of PSM should strictly require STS, and if STS init fails, then all security/SSL should fail, too, you could enhance your patch in a way similar to bug 619201.

Can we get this landed?

We apparently have many places where PSM accesses services from secondary threads, this isn't new code. A recent change had introduced assertions/warnings for such scenarios, and it caused drivers to mark this bug as blocking+. However, that constraining code has now been backed out - because it apparently had caused too many problems. So, at this time, it might be unnecessary to land this code. In addition, I had proposed to improve the current patch, see comment 13. I think it's up to Sid whether he wants to land this code, or not. Maybe it's better to wait for a better plan? The similar bug 619201 has not been landed either, it has actually been rejected by beltzner.

(In reply to comment #15) > A recent change had introduced assertions/warnings for such scenarios, and it > caused drivers to mark this bug as blocking+. > > However, that constraining code has now been backed out - because it apparently > had caused too many problems. Isn't this referring to the use of the pref service off the main thread? That's an orthogonal issue to this one.

not waiting for this

Comment on attachment 501754 [details] [diff] [review] proposed patch This patch is not correct. It has the same problem as bug 650858.

This will be fixed when the patch in bug 679036 lands.

Works for me after the patch for bug 679140 was checked in. I verified this by going to https://www.cacert.org immediately after opening the browser in a debug build. Before the patch for bug 679140, I would get the assertions about the permission service being used off the main thread. After the patch for bug bug 679140, I do not get any such assertions.

Can we put an assertion in nsStrictTransportSecurityService's ctor that asserts it's being constructed on the main thread so that we don't have to rely on the permissions manager yelling at us to see this problem?

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #21) > Can we put an assertion in nsStrictTransportSecurityService's ctor that > asserts it's being constructed on the main thread so that we don't have to > rely on the permissions manager yelling at us to see this problem? For posterity, this was addressed in bug 775370.