Last Comment Bug 619201 - nsCertOverrideService needs to initialize on the main thread to use the directory service
: nsCertOverrideService needs to initialize on the main thread to use the direc...
Product: Core
Classification: Components
Component: Security: PSM (show other bugs)
: unspecified
: x86 Linux
-- normal (vote)
: mozilla7
Assigned To: timeless
: David Keeler [:keeler] (use needinfo?)
Depends on: 650858 650892
  Show dependency treegraph
Reported: 2010-12-14 15:19 PST by Josh Matthews [:jdm]
Modified: 2011-10-30 22:28 PDT (History)
11 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

add to the ball of wax (1.14 KB, patch)
2010-12-15 07:23 PST, timeless
kaie: review+
mbeltzner: approval2.0-
Details | Diff | Splinter Review

Description User image Josh Matthews [:jdm] 2010-12-14 15:19:13 PST
Running Fennec on a restricted wireless network currently brings up an SSL cert error which triggers this assertion:

#0  RealBreak () at /home/t_mattjo/src/firefox/mobilebase/xpcom/base/nsDebugImpl.cpp:407
#1  0x0258d684 in Break (aMsg=
    0xb00fe35c "###!!! ASSERTION: mozHunspellDirProvider not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file /home/t_mattjo/src/firefox/mobilebase/extensions/spellcheck/hunspell/src/mozHunspe"...) at /home/t_mattjo/src/firefox/mobilebase/xpcom/base/nsDebugImpl.cpp:503
#2  0x0258d649 in NS_DebugBreak_P (aSeverity=1, aStr=0x30169e8 "mozHunspellDirProvider not thread-safe", aExpr=0x30169b4 "_mOwningThread.GetThread() == PR_GetCurrentThread()", aFile=
    0x30168cc "/home/t_mattjo/src/firefox/mobilebase/extensions/spellcheck/hunspell/src/mozHunspellDirProvider.cpp", aLine=45) at /home/t_mattjo/src/firefox/mobilebase/xpcom/base/nsDebugImpl.cpp:371
#3  0x022f7367 in mozHunspellDirProvider::AddRef (this=0xb40e01c0) at /home/t_mattjo/src/firefox/mobilebase/extensions/spellcheck/hunspell/src/mozHunspellDirProvider.cpp:43
#4  0x02508df3 in NS_TableDrivenQI (aThis=0xb40e01c0, entries=0x328db20, aIID=..., aInstancePtr=0xb00fe86c) at nsISupportsImpl.cpp:49
#5  0x022f75ec in mozHunspellDirProvider::QueryInterface (this=0xb40e01c0, aIID=..., aInstancePtr=0xb00fe86c)
    at /home/t_mattjo/src/firefox/mobilebase/extensions/spellcheck/hunspell/src/mozHunspellDirProvider.cpp:43
#6  0x025053c9 in nsQueryInterface::operator() (this=0xb00fe884, aIID=..., answer=0xb00fe86c) at nsCOMPtr.cpp:47
#7  0x00ff5136 in nsCOMPtr<nsIDirectoryServiceProvider>::assign_from_qi (this=0xb00fe8c8, qi=..., aIID=...) at ../../dist/include/nsCOMPtr.h:1212
#8  0x00ff494c in nsCOMPtr<nsIDirectoryServiceProvider>::nsCOMPtr (this=0xb00fe8c8, qi=...) at ../../dist/include/nsCOMPtr.h:595
#9  0x02548f6a in FindProviderFile (aElement=0xb40e01c0, aData=0xb00fe974) at /home/t_mattjo/src/firefox/mobilebase/xpcom/io/nsDirectoryService.cpp:433
#10 0x02536465 in nsSupportsArray::EnumerateBackwards (this=0xb7dd0d00, aFunc=0x2548d18 <FindProviderFile(nsISupports*, void*)>, aData=0xb00fe974)
    at /home/t_mattjo/src/firefox/mobilebase/xpcom/ds/nsSupportsArray.cpp:639
#11 0x0254922a in nsDirectoryService::Get (this=0xb7de5100, prop=0x2f3440c "ProfD", uuid=..., result=0xabb2d0cc) at /home/t_mattjo/src/firefox/mobilebase/xpcom/io/nsDirectoryService.cpp:468
#12 0x00fea1df in NS_GetSpecialDirectory (specialDirName=0x2f3440c "ProfD", result=0xabb2d0cc) at ../../dist/include/nsDirectoryServiceUtils.h:58
#13 0x01fc5f97 in nsCertOverrideService::Init (this=0xabb2d0b0) at /home/t_mattjo/src/firefox/mobilebase/security/manager/ssl/src/nsCertOverrideService.cpp:152
#14 0x01ff0881 in nsCertOverrideServiceConstructor (aOuter=0x0, aIID=..., aResult=0xb00feb38) at /home/t_mattjo/src/firefox/mobilebase/security/manager/ssl/src/nsNSSModule.cpp:259
#15 0x02514d28 in mozilla::GenericFactory::CreateInstance (this=0xae173750, aOuter=0x0, aIID=..., aResult=0xb00feb38) at GenericFactory.cpp:48
#16 0x0257244f in nsComponentManagerImpl::CreateInstanceByContractID (this=0xb7dc3220, aContractID=0x2f3c3d4 ";1", aDelegate=0x0, aIID=..., aResult=0xb00feb38)
    at /home/t_mattjo/src/firefox/mobilebase/xpcom/components/nsComponentManager.cpp:1315
#17 0x025730c2 in nsComponentManagerImpl::GetServiceByContractID (this=0xb7dc3220, aContractID=0x2f3c3d4 ";1", aIID=..., result=0xb00fec2c)
    at /home/t_mattjo/src/firefox/mobilebase/xpcom/components/nsComponentManager.cpp:1676
#18 0x025063a2 in CallGetService (aContractID=0x2f3c3d4 ";1", aIID=..., aResult=0xb00fec2c) at nsComponentManagerUtils.cpp:94
#19 0x02506890 in nsGetServiceByContractID::operator() (this=0xb00fec44, aIID=..., aInstancePtr=0xb00fec2c) at nsComponentManagerUtils.cpp:278
#20 0x01fdfb48 in nsCOMPtr<nsICertOverrideService>::assign_from_gs_contractid (this=0xb00fed70, gs=..., aIID=...) at ../../../../dist/include/nsCOMPtr.h:1252
#21 0x01fdf504 in nsCOMPtr<nsICertOverrideService>::nsCOMPtr (this=0xb00fed70, gs=...) at ../../../../dist/include/nsCOMPtr.h:627
#22 0x01fe9efa in nsNSSBadCertHandler (arg=0xb058f710, sslSocket=0xae1bf0e0) at /home/t_mattjo/src/firefox/mobilebase/security/manager/ssl/src/nsNSSIOLayer.cpp:3519
#23 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
#24 0x0036c49a in ssl3_HandleHandshakeMessage (ss=0xae162000, b=0xae1a0004 "", length=900) at ssl3con.c:8603
#25 0x0036c928 in ssl3_HandleHandshake (ss=0xae162000, origBuf=0xae162250) at ssl3con.c:8727
#26 0x0036d40a in ssl3_HandleRecord (ss=0xae162000, cText=0xb00ff08c, databuf=0xae162250) at ssl3con.c:9066
#27 0x0036e527 in ssl3_GatherCompleteHandshake (ss=0xae162000, flags=0) at ssl3gthr.c:209
#28 0x0037117b in ssl_GatherRecord1stHandshake (ss=0xae162000) at sslcon.c:1258
#29 0x0037c824 in ssl_Do1stHandshake (ss=0xae162000) at sslsecur.c:151
#30 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:\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
#31 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:\r\nUser-Agent: Mozilla/5.0 (X11; Linux i686; rv:2.0b8pre) Gecko/20101213 Firefox/4.0b8pre Fennec/"..., len=631) at sslsecur.c:1258
#32 0x0038628f in ssl_Write (fd=0xae1bf0e0, buf=0xae110000, len=631) at sslsock.c:1652
#33 0x01fccf3c in nsSSLThread::Run (this=0xb01705e0) at /home/t_mattjo/src/firefox/mobilebase/security/manager/ssl/src/nsSSLThread.cpp:1045
#34 0x01fcb26b in nsPSMBackgroundThread::nsThreadRunner (arg=0xb01705e0) at /home/t_mattjo/src/firefox/mobilebase/security/manager/ssl/src/nsPSMBackgroundThread.cpp:44
---Type <return> to continue, or q <return> to quit---
#35 0x00182797 in _pt_root (arg=0xb037fa80) at /home/t_mattjo/src/firefox/mobilebase/nsprpub/pr/src/pthreads/ptthread.c:187
#36 0x008ba925 in start_thread () from /lib/
#37 0x007e407e in clone () from /lib/
(gdb) fr 13
#13 0x01fc5f97 in nsCertOverrideService::Init (this=0xabb2d0b0) at /home/t_mattjo/src/firefox/mobilebase/security/manager/ssl/src/nsCertOverrideService.cpp:152
152	  NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, getter_AddRefs(mSettingsFile));
(gdb) list
148	  mDottedOidForStoringNewHashes = dotted_oid;
149	  PR_smprintf_free(dotted_oid);
151	  // cache mSettingsFile
152	  NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, getter_AddRefs(mSettingsFile));
153	  if (mSettingsFile) {
154	    mSettingsFile->AppendNative(NS_LITERAL_CSTRING(kCertOverrideFileName));
155	  }
Comment 1 User image Benjamin Smedberg [:bsmedberg] 2010-12-14 16:33:41 PST
Yeah, quite clearly nsCertOverrideService::Init should not be calling into the directory service off the main thread. This can be a cause of crashes. The possibilities are:

* We should pre-create nsCertOverrideService on the main thread
* We should not be creating the nsCertOverrideService off the main thread
* ?

Who knows the code?
Comment 2 User image timeless 2010-12-15 07:23:48 PST
Created attachment 497777 [details] [diff] [review]
add to the ball of wax
Comment 3 User image Kai Engert (:kaie) 2011-01-10 07:11:35 PST
Comment on attachment 497777 [details] [diff] [review]
add to the ball of wax

Is it sufficent to create the service once, because XPCOM will keep it alive?
If yes, r=kaie

Or will the service be deleted on leaving the scope, and will need to be constructed again later?
If yes, r- and please add a member variable nsNSSComponent::mCertOverrServ to keep a reference.
Comment 4 User image Kai Engert (:kaie) 2011-01-10 09:23:11 PST
question has been answered, once a service is alive, XPCOM will keep it alive until shutdown.

patch is sufficient.
Comment 5 User image Mike Beltzner [:beltzner, not reading bugmail] 2011-02-16 12:04:41 PST
Comment on attachment 497777 [details] [diff] [review]
add to the ball of wax

If Fennec wants this, Fennec drivers can ask for it.
Comment 7 User image Johnny Stenback (:jst, 2011-06-13 15:50:06 PDT
Backed out of the beta channel due to bug 650858.
Comment 8 User image Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2011-07-19 17:41:47 PDT
If we implement the design in bug 511393 comment 14, then we can make nsICertOverrideService main-thread only, which will provide an alternate solution for this bug. 

Besides causing bug 650858, the current solution to this bug has the added disadvantage of adding disk I/O on the main thread at startup. So, my recommendation is to backout this patch on all trees (including mozilla-central) and then fix bug 511393.

If backing out the patch is problematic (i.e. we have some evidence that it really does cause crashes), then I will hack together an alternate fix for this bug that avoids causing bug 650858.
Comment 9 User image christian 2011-08-03 17:16:44 PDT
I backed this out of beta6 too:

So bug 650858 doesn't affect it anymore. I have not backed it out of aurora or central.
Comment 10 User image christian 2011-08-16 00:10:25 PDT
Also backed out of aurora7:
Comment 11 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-10-13 09:49:34 PDT
qa- as QA does not need to verify this fix.
Comment 12 User image Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2011-10-30 22:28:36 PDT
Also backed out of beta-8:

The patch for bug 679140 fixes this in a different way, by ensuring that we only use the cert override service on the main thread, so that it is guaranteed to be initialized on the main thread without having to do so in nsNSSComponent::Init().

Note You need to log in before you can comment on or make changes to this bug.