399304 - cert trust lock and refcount lock should not be initialized late

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Slavomir Katuscak]

Yesterday I enabled testing with libpkix on 4 tinderboxes. Today I found 3 of them (2x Solaris, 1x Windows) with strsclnt hanged.

I attached one on Solaris to DBX, and here is the stack:

Attached to process 12353 with 9 LWPs
t@1 (l@1) stopped in __lwp_wait at 0xbf89df87
0xbf89df87: __lwp_wait+0x0007:  jae      __lwp_wait+0x15        [ 0xbf89df95, .+0xe ]
Current function is PR_JoinThread
  592           rv = pthread_join(id, &result);
(dbx) where
current thread: t@1
  [1] __lwp_wait(0x2, 0x8046e10), at 0xbf89df87
  [2] lwp_wait(0x2, 0x8046e10), at 0xbf8914d3
  [3] _thrp_join(0x2, 0x0, 0x8046e58, 0x1), at 0xbf89a1f0
  [4] _pthread_join(0x2, 0x8046e58), at 0xbf89a36f
=>[5] PR_JoinThread(thred = 0x80bfc78), line 592 in "ptthread.c"
  [6] reap_threads(), line 500 in "strsclnt.c"
  [7] client_main(port = 8443U, connections = 10, Cert_And_Key = 0x8046fa0, hostName = 0x8083be8 "dositups.red.iplanet.com"), line 1275 in "strsclnt.c"
  [8] main(argc = 18, argv = 0x8047018), line 1469 in "strsclnt.c"

Comment 1

[Alexei Volkov]

All other threads are yielding with the stack below.

=>[1] lwp_yield(0xbfc56000, 0xbf01d10c, 0xbfc44ddb, 0xbfd7372c, 0xbf01d138, 0xbfd5c12f), at 0xbfbfd307
  [2] _thr_yield(0xbfd7372c, 0xbf01d138, 0xbfd5c12f, 0xbff1c560, 0xbff24a20, 0xbff1b6a0), at 0xbfbfb6eb
  [3] sched_yield(0xbff1c560, 0xbff24a20, 0xbff1b6a0, 0x820eff0, 0xbfd42e53, 0xbff24a20), at 0xbfc44ddb
  [4] PR_Sleep(0x0, 0x820eefc, 0x81fe808, 0xbff1b6a0, 0x820eff0, 0x0), at 0xbfd5c12f
  [5] nss_InitLock(0xbff1c560, 0x5), at 0xbfe3e791
  [6] CERT_LockCertTrust(0x81fe808), at 0xbfe2cb4a
  [7] CERT_GetCertTrust(0x81fe808, 0xbf01d1e4), at 0xbfe3547a
  [8] pkix_pl_Pk11CertStore_CheckTrust(0x820685c, 0x820eefc, 0xbf01d284, 0x80b87e8), at 0xbfef115e
  [9] PKIX_PL_Cert_IsCertTrusted(0x820eefc, 0xbf01d308, 0x80b87e8), at 0xbfeb3007
  [10] pkix_Build_VerifyCertificate(0x821c36c, 0x0, 0x1, 0xbf01d44c, 0xbf01d458, 0x821c134, 0x80b87e8), at 0xbfe8b088
  [11] pkix_BuildForwardDepthFirstSearch(0xbf01d5f0, 0xbf01d604, 0xbf01d5f8, 0x80b87e8), at 0xbfe8e231
  [12] pkix_Build_InitiateBuildChain(0x82128ac, 0xbf01d68c, 0xbf01d684, 0xbf01d688, 0xbf01d758, 0x80b87e8), at 0xbfe92729
  [13] PKIX_BuildChain(0x82128ac, 0xbf01d75c, 0xbf01d760, 0xbf01d754, 0xbf01d758, 0x80b87e8), at 0xbfe9326e
  [14] cert_VerifyCertChainPkix(0x81fce98, 0x1, 0x1, 0xf4df91b0, 0x43c26, 0x0, 0x0, 0x0, 0x0), at 0xbfdf192f
  [15] CERT_VerifyCert(0x80b88d8, 0x81fce98, 0x1, 0x1, 0xf4df91b0, 0x43c26, 0x0, 0x0), at 0xbfded9a2
  [16] CERT_VerifyCertNow(0x80b88d8, 0x81fce98, 0x1, 0x1, 0x0), at 0xbfdede83
  [17] SSL_AuthCertificate(0x80b88d8, 0x80c3ec0, 0x1, 0x0), at 0xbff966e9
  [18] mySSLAuthCertificate(0x80b88d8, 0x80c3ec0, 0x1, 0x0), at 0x8054d22
  [19] ssl3_HandleCertificate(0x81568c0, 0x815ea4b, 0x0), at 0xbff92a75
  [20] ssl3_HandleHandshakeMessage(0x81568c0, 0x815e05e, 0x9ed), at 0xbff94541
  [21] ssl3_HandleHandshake(0x81568c0, 0x8156b1c), at 0xbff946ef
  [22] ssl3_HandleRecord(0x81568c0, 0xbf01da24, 0x8156b1c), at 0xbff94e16
  [23] ssl3_GatherCompleteHandshake(0x81568c0, 0x0), at 0xbff961e8
  [24] ssl_GatherRecord1stHandshake(0x81568c0), at 0xbff97807
  [25] ssl_Do1stHandshake(0x81568c0), at 0xbff9d6e3
  [26] ssl_SecureSend(0x81568c0, 0x8068a14, 0x15, 0x0), at 0xbff9f761
  [27] ssl_Send(0x80c3ec0, 0x8068a14, 0x15, 0x0, 0xffffffff), at 0xbffa4be8
  [28] PR_Send(0x80c3ec0, 0x8068a14, 0x15, 0x0, 0xffffffff), at 0xbfd3a66a
  [29] do_connects(0x8046eb4, 0x8083c00, 0x7), at 0x8056632
  [30] thread_wrapper(0x8081a34), at 0x805515c
  [31] _pt_root(0x80d7410), at 0xbfd5b341
  [32] _thr_setup(0xbf521800), at 0xbfbfcf2f
  [33] _lwp_start(), at 0xbfbfd220

Comment 2

[Slavomir Katuscak]

Found 9 threads together, 1 with stack from description and 8 with this:
  [1] lwp_yield(0x0, 0x5, 0x0, 0xfffffd7ffe8f7e7a, 0x0, 0xfffffd7fff0bcbd8), at 0xfffffd7ffe8f7e7a 
  [2] _thr_yield(0x0, 0x0, 0x0, 0x0, 0x0, 0x0), at 0xfffffd7ffe8f5f79 
  [3] sched_yield(0x0, 0x0, 0x0, 0x0, 0x0, 0x0), at 0xfffffd7fff126709 
=>[4] PR_Sleep(ticks = 0), line 785 in "ptthread.c"
  [5] __nss_InitLock(ppLock = 0xfffffd7fff0bcbd8, ltype = nssILockCertDB), line 72 in "nsslocks.c"
  [6] nss_InitLock(ppLock = 0xfffffd7fff0bcbd8, ltype = nssILockCertDB), line 82 in "nsslocks.c"
  [7] CERT_LockCertTrust(cert = 0x5ecd50), line 2664 in "certdb.c"
  [8] CERT_GetCertTrust(cert = 0x5ecd50, trust = 0xfffffd7ffd3fc8fc), line 115 in "stanpcertdb.c"
  [9] pkix_pl_Pk11CertStore_CheckTrust(store = 0x5f56d8, cert = 0x604ba8, pTrusted = 0xfffffd7ffd3fca20, plContext = 0x4ae5c0), line 104 in "pkix_pl_pk11certstore.c"
  [10] PKIX_PL_Cert_IsCertTrusted(cert = 0x604ba8, pTrusted = 0xfffffd7ffd3fcb1c, plContext = 0x4ae5c0), line 3252 in "pkix_pl_cert.c"
  [11] pkix_Build_VerifyCertificate(state = 0x633de8, userCheckers = (nil), revocationChecking = 1, pTrusted = 0xfffffd7ffd3fcd20, pNeedsCRLChecking = 0xfffffd7ffd3fcd0c, verifyNode = 0x6349d8, plContext = 0x4ae5c0), line 1075 in "pkix_build.c"
  [12] pkix_BuildForwardDepthFirstSearch(pNBIOContext = 0xfffffd7ffd3fced0, pState = 0xfffffd7ffd3fce30, pValResult = 0xfffffd7ffd3fce50, plContext = 0x4ae5c0), line 2734 in "pkix_build.c"
  [13] pkix_Build_InitiateBuildChain(procParams = 0x618918, pNBIOContext = 0xfffffd7ffd3fd040, pState = 0xfffffd7ffd3fd050, pBuildResult = 0xfffffd7ffd3fd048, pVerifyNode = 0xfffffd7ffd3fd148, plContext = 0x4ae5c0), line 4181 in "pkix_build.c"
  [14] PKIX_BuildChain(procParams = 0x618918, pNBIOContext = 0xfffffd7ffd3fd140, pState = 0xfffffd7ffd3fd138, pBuildResult = 0xfffffd7ffd3fd150, pVerifyNode = 0xfffffd7ffd3fd148, plContext = 0x4ae5c0), line 4364 in "pkix_build.c"
  [15] cert_BuildAndValidateChain(procParams = 0x618918, pResult = 0xfffffd7ffd3fd1f0, pVerifyNode = 0xfffffd7ffd3fd1e8, plContext = 0x4ae5c0), line 755 in "certvfypkix.c"
  [16] cert_VerifyCertChainPkix(cert = 0x4af560, checkSig = 1, requiredUsage = certUsageSSLServer, time = 1192115086273139U, wincx = (nil), log = (nil), pSigerror = (nil), pRevoked = (nil)), line 1142 in "certvfypkix.c"
  [17] cert_VerifyCertChain(handle = 0x494600, cert = 0x4af560, checkSig = 1, sigerror = (nil), certUsage = certUsageSSLServer, t = 1192115086273139, wincx = (nil), log = (nil), revoked = (nil)), line 946 in "certvfy.c"
  [18] CERT_VerifyCertChain(handle = 0x494600, cert = 0x4af560, checkSig = 1, certUsage = certUsageSSLServer, t = 1192115086273139, wincx = (nil), log = (nil)), line 958 in "certvfy.c"
  [19] CERT_VerifyCert(handle = 0x494600, cert = 0x4af560, checkSig = 1, certUsage = certUsageSSLServer, t = 1192115086273139, wincx = (nil), log = (nil)), line 1556 in "certvfy.c"
  [20] CERT_VerifyCertNow(handle = 0x494600, cert = 0x4af560, checkSig = 1, certUsage = certUsageSSLServer, wincx = (nil)), line 1607 in "certvfy.c"
  [21] SSL_AuthCertificate(arg = 0x494600, fd = 0x443f30, checkSig = 1, isServer = 0), line 255 in "sslauth.c"
  [22] mySSLAuthCertificate(arg = 0x494600, fd = 0x443f30, checkSig = 1, isServer = 0), line 280 in "strsclnt.c"
  [23] ssl3_HandleCertificate(ss = 0x5b4730, b = 0x5bc8dc "^M", length = 0), line 7120 in "ssl3con.c"
  [24] ssl3_HandleHandshakeMessage(ss = 0x5b4730, b = 0x5bc3de "", length = 1278U), line 7782 in "ssl3con.c"
  [25] ssl3_HandleHandshake(ss = 0x5b4730, origBuf = 0x5b4ab0), line 7898 in "ssl3con.c"
  [26] ssl3_HandleRecord(ss = 0x5b4730, cText = 0xfffffd7ffd3fd898, databuf = 0x5b4ab0), line 8161 in "ssl3con.c"
  [27] ssl3_GatherCompleteHandshake(ss = 0x5b4730, flags = 0), line 206 in "ssl3gthr.c"
  [28] ssl_GatherRecord1stHandshake(ss = 0x5b4730), line 1258 in "sslcon.c"
  [29] ssl_Do1stHandshake(ss = 0x5b4730), line 151 in "sslsecur.c"
  [30] ssl_SecureSend(ss = 0x5b4730, buf = 0x41ef08 "GET /abc HTTP/1.0^M\n^M\n", len = 21, flags = 0), line 1152 in "sslsecur.c"
  [31] ssl_Send(fd = 0x443f30, buf = 0x41ef08, len = 21, flags = 0, timeout = 4294967295U), line 1432 in "sslsock.c"
  [32] PR_Send(fd = 0x443f30, buf = 0x41ef08, amount = 21, flags = 0, timeout = 4294967295U), line 226 in "priometh.c"
  [33] handle_connection(ssl_sock = 0x443f30, tid = 7), line 696 in "strsclnt.c"
  [34] do_connects(a = 0xfffffd7fffdfec90, b = 0x443970, tid = 7), line 887 in "strsclnt.c"
  [35] thread_wrapper(arg = 0x43b8d0), line 439 in "strsclnt.c"
  [36] _pt_root(arg = 0x4d1fa0), line 221 in "ptthread.c"
  [37] _thr_setup(0x0, 0x0, 0x0, 0x0, 0x0, 0x0), at 0xfffffd7ffe8f7b4b 
  [38] _lwp_start(0x0, 0x0, 0x0, 0x0, 0x0, 0x0), at 0xfffffd7ffe8f7d80 

Comment 3

[Slavomir Katuscak]

I created workaround for this bug for Tinderboxes (it's already running for some days). There is script on one machine (mace) checking every hour PIDs of running selfservs and strsclnts on Tinderbox machines. If PID is the same as it was hour before, the script kills it (to prevent Tinderboxes hangs). You can easy detect this in log, when you can see Terminated message and the time between start and termination is more then one hour (and minutes at termination time are 00).

Comment 4

[Julien Pierre]

Slavo,

How often is this happening on the tinderbox ? Do you record when your workaround kills the process ? If not, please do.
I don't see any evidence in the recent tinderbox run that the process was killed - nearly everything is green for the PKIX runs.

I am asking this because I am unable to get any run of all.sh with PKIX in optimized mode on my Linux AMD . It always hangs in strsclnt.

The stacks point to a long-standig race condition with trust, but it must be only triggered by the PKIX code. We should never do late initialization of locks. This is recipe for disaster. I will create a patch and see if it fixes the problem for me, then attach it.

Taking bug.

Comment 5

[Julien Pierre]

Attachment: Initialize cert and trust locks early

Comment 6

[Robert Relyea]

Comment on attachment 287791 [details] [diff] [review]
Initialize cert and trust locks early

r+ rrelyea

comment... fix the tabs in cert_InitLocks().

These locks clearly can be initialized at startup, if we have locks that need to be lazy initialized we should use PR_CallOnce().

bob

Comment 7

[Wan-Teh Chang]

Does this only happen in optimized builds?  If so, would be
nice to examine the assembly code of the while loop in
__nss_InitLock.

You can try declaring what ppLock points to as volatile:

PZLock * volatile *ppLock

Comment 8

[Julien Pierre]

Wan-Teh,

I have only seen this problem in optimized builds indeed. But it has happened on multiple types of CPUs. It sounds like Slavo has seen it in some debug builds too.

I reviewed the source __nss_InitLock and I can't really find anything wrong with it, except for CPUs without total store order, but that would only be possible on AIX (PowerPC), and I saw the problem happened on Linux amd64 and Slavo on Windows x86 . Slavo also saw it on Solaris but didn't specify which CPU type.

The patch I attached resolved the hang problem for me on Linux amd64 however.

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 287791 [details] [diff] [review]
Initialize cert and trust locks early

r+, with some suggested changes.

>@@ -2733,13 +2729,51 @@
> void
> CERT_LockCertTrust(CERTCertificate *cert)
> {
>+    PORT_Assert(certTrustLock != NULL);
>+    PZ_Lock(certTrustLock);
>+    return;
>+}
>+
>+SECStatus cert_InitLocks(void)

I suggest you make this function match the style used in the
other function definitions in this source file, where the 
return type is defined on a separate line before the file name.

>+{
>+    if ( certRefCountLock == NULL ) {
>+	nss_InitLock(&certRefCountLock, nssILockRefLock);
>+	PORT_Assert(certRefCountLock != NULL);
>+        if (!certRefCountLock) {
>+            return SECFailure;
>+        }
>+    }
>+
>     if ( certTrustLock == NULL ) {
> 	nss_InitLock(&certTrustLock, nssILockCertDB);
> 	PORT_Assert(certTrustLock != NULL);
>+        if (!certTrustLock) {
>+            PZ_DestroyLock(certRefCountLock);

I suggest you set certRefCountLock to NULL here to avoid 
any possibility of it being destroyed a second time in 
NSS_Shutdown.

>+            return SECFailure;
>+        }
>+    }    
>+
>+    return SECSuccess;
>+}
>+
>+SECStatus cert_DestroyLocks(void)

This function also does not follow the style convention.

Comment 11

[Julien Pierre]

Bob, Nelson, thanks for the reviews.

I checked this in to the trunk :

Checking in certdb/certdb.c;
/cvsroot/mozilla/security/nss/lib/certdb/certdb.c,v  <--  certdb.c
new revision: 1.83; previous revision: 1.82
done
Checking in certdb/certi.h;
/cvsroot/mozilla/security/nss/lib/certdb/certi.h,v  <--  certi.h
new revision: 1.22; previous revision: 1.21
done
Checking in nss/nssinit.c;
/cvsroot/mozilla/security/nss/lib/nss/nssinit.c,v  <--  nssinit.c
new revision: 1.85; previous revision: 1.84
done

and to NSS_3_11_BRANCH :

Checking in certdb/certdb.c;
/cvsroot/mozilla/security/nss/lib/certdb/certdb.c,v  <--  certdb.c
new revision: 1.76.2.2; previous revision: 1.76.2.1
done
Checking in certdb/certi.h;
/cvsroot/mozilla/security/nss/lib/certdb/certi.h,v  <--  certi.h
new revision: 1.16.24.1; previous revision: 1.16
done
Checking in nss/nssinit.c;
/cvsroot/mozilla/security/nss/lib/nss/nssinit.c,v  <--  nssinit.c
new revision: 1.69.2.8; previous revision: 1.69.2.7
done

Comment 12

[Wan-Teh Chang]

Comment on attachment 287791 [details] [diff] [review]
Initialize cert and trust locks early

In certdb.c

>+	nss_InitLock(&certRefCountLock, nssILockRefLock);

This should be changed to

    certRefCountLock = PZ_NewLock(nssILockRefLock);

and similarly for the other nss_InitLock call
in the cert_InitLocks function.

Comment 13

[Julien Pierre]

Wan-Teh,

Indeed. I proposed doing this accross the board in NSS in bug 403240 comment 1, item 2 .

Even after backing out my fix, I can't reproduce the hang on Linux amd optimized right now. Just my luck. And the disassembly from gdb is weird.

I believe it's the right instructions, but all the symbol names for the call instructions are wrong. I don't understand why.

This is the output without using the volatile keyword.

(gdb) disassemble __nss_InitLock_Util
Dump of assembler code for function __nss_InitLock_Util:
0x0000002a959fc460 <__nss_InitLock_Util+0>:     push   %rbx
0x0000002a959fc461 <__nss_InitLock_Util+1>:     mov    %rdi,%rbx
0x0000002a959fc464 <__nss_InitLock_Util+4>:     cmpq   $0x0,(%rbx)
0x0000002a959fc468 <__nss_InitLock_Util+8>:     jne    0x2a959fc499 <__nss_InitLock_Util+57>
0x0000002a959fc46a <__nss_InitLock_Util+10>:    data16
0x0000002a959fc46b <__nss_InitLock_Util+11>:    data16
0x0000002a959fc46c <__nss_InitLock_Util+12>:    nop
0x0000002a959fc46d <__nss_InitLock_Util+13>:    data16
0x0000002a959fc46e <__nss_InitLock_Util+14>:    data16
0x0000002a959fc46f <__nss_InitLock_Util+15>:    nop
0x0000002a959fc470 <__nss_InitLock_Util+16>:    lea    1116081(%rip),%rdi        # 0x2a95b0cc28 <initializers.1>
0x0000002a959fc477 <__nss_InitLock_Util+23>:    callq  0x2a959f7f38 <PORT_Strdup_Util@plt+48>
0x0000002a959fc47c <__nss_InitLock_Util+28>:    dec    %eax
0x0000002a959fc47e <__nss_InitLock_Util+30>:    je     0x2a959fc49d <__nss_InitLock_Util+61>
0x0000002a959fc480 <__nss_InitLock_Util+32>:    xor    %edi,%edi
0x0000002a959fc482 <__nss_InitLock_Util+34>:    callq  0x2a959f8048 <PORT_ArenaRelease_Util@plt+32>
0x0000002a959fc487 <__nss_InitLock_Util+39>:    lea    1116058(%rip),%rdi        # 0x2a95b0cc28 <initializers.1>
0x0000002a959fc48e <__nss_InitLock_Util+46>:    callq  0x2a959f81e8 <SEC_ASN1DecoderFinish_Util@plt+64>
0x0000002a959fc493 <__nss_InitLock_Util+51>:    cmpq   $0x0,(%rbx)
0x0000002a959fc497 <__nss_InitLock_Util+55>:    je     0x2a959fc470 <__nss_InitLock_Util+16>
0x0000002a959fc499 <__nss_InitLock_Util+57>:    xor    %eax,%eax
0x0000002a959fc49b <__nss_InitLock_Util+59>:    pop    %rbx
0x0000002a959fc49c <__nss_InitLock_Util+60>:    retq
0x0000002a959fc49d <__nss_InitLock_Util+61>:    cmpq   $0x0,(%rbx)
0x0000002a959fc4a1 <__nss_InitLock_Util+65>:    je     0x2a959fc4be <__nss_InitLock_Util+94>
0x0000002a959fc4a3 <__nss_InitLock_Util+67>:    lea    1116030(%rip),%rdi        # 0x2a95b0cc28 <initializers.1>
0x0000002a959fc4aa <__nss_InitLock_Util+74>:    callq  0x2a959f81e8 <SEC_ASN1DecoderFinish_Util@plt+64>
0x0000002a959fc4af <__nss_InitLock_Util+79>:    cmpq   $0x0,(%rbx)
0x0000002a959fc4b3 <__nss_InitLock_Util+83>:    mov    $0xffffffff,%eax
0x0000002a959fc4b8 <__nss_InitLock_Util+88>:    je     0x2a959fc49b <__nss_InitLock_Util+59>
0x0000002a959fc4ba <__nss_InitLock_Util+90>:    xor    %eax,%eax
---Type <return> to continue, or q <return> to quit---
0x0000002a959fc4bc <__nss_InitLock_Util+92>:    jmp    0x2a959fc49b <__nss_InitLock_Util+59>
0x0000002a959fc4be <__nss_InitLock_Util+94>:    data16
0x0000002a959fc4bf <__nss_InitLock_Util+95>:    nop
0x0000002a959fc4c0 <__nss_InitLock_Util+96>:    callq  0x2a959f84b8 <SEC_ASN1DecoderStart_Util@plt+32>
0x0000002a959fc4c5 <__nss_InitLock_Util+101>:   mov    %rax,(%rbx)
0x0000002a959fc4c8 <__nss_InitLock_Util+104>:   jmp    0x2a959fc4a3 <__nss_InitLock_Util+67>
0x0000002a959fc4ca <__nss_InitLock_Util+106>:   data16
0x0000002a959fc4cb <__nss_InitLock_Util+107>:   data16
0x0000002a959fc4cc <__nss_InitLock_Util+108>:   nop
0x0000002a959fc4cd <__nss_InitLock_Util+109>:   data16
0x0000002a959fc4ce <__nss_InitLock_Util+110>:   data16
0x0000002a959fc4cf <__nss_InitLock_Util+111>:   nop
End of assembler dump.
(gdb)

Comment 14

[Wan-Teh Chang]

Even if nss_InitLock is correct, cert_InitLocks doesn't
need to call nss_InitLock because cert_InitLocks is called
during NSS initialization, which is performed before any
other NSS function can be called.

Comment 15

[Nelson Bolyard (seldom reads bugmail)]

Julien, maybe that disassembly was made by a 32-bit debugger on 64-bit code?
Those data16's are bogus.

Comment 16

[Julien Pierre]

Nelson,

Re: comment 15, it was a 64-bit debugger.

[jp96085@nssamdrhel4]/net/monstre/export/home/julien/nss/tip/mozilla/dist/Linux2.6_x86_64_glibc_PTH_64_OPT.OBJ/bin 40 % file /usr/bin/gdb
/usr/bin/gdb: ELF 64-bit LSB executable, AMD x86-64, version 1 (SYSV), for GNU/Linux 2.4.0, dynamically linked (uses shared libs), stripped

Comment 17

[Wan-Teh Chang]

Attachment: Use PZ_NewLock instead of nss_InitLock

Julien, you don't need to examine the assembly code or experiment with
volatile as I suggested in comment 7.  I figured out why nss_InitLock
is broken in bug 403240 comment 7.

This patch implements my suggested changes in comment 12.  Please review.

Comment 18

[Wan-Teh Chang]

Comment on attachment 288517 [details] [diff] [review]
Use PZ_NewLock instead of nss_InitLock

I checked in this patch on the NSS trunk (NSS 3.12).

Comment 19

[Julien Pierre]

Reopening since a fix is still pending for the branch.

Comment 20

[Julien Pierre]

Comment on attachment 288517 [details] [diff] [review]
Use PZ_NewLock instead of nss_InitLock

Nelson, please review this patch for the 3.11 branch. Thanks.

Comment 21

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 288517 [details] [diff] [review]
Use PZ_NewLock instead of nss_InitLock

This patch is as correct on the branch as it is on the trunk.

Comment 22

[Julien Pierre]

Thanks, Nelson. I checked this patch in to NSS_3_11_BRANCH .

Checking in certdb.c;
/cvsroot/mozilla/security/nss/lib/certdb/certdb.c,v  <--  certdb.c
new revision: 1.76.2.3; previous revision: 1.76.2.2
done

I will create a separate bug to do the same thing in the other places where nss_InitLock is currently called but PZ_NewLock will do just as well.