diff --git a/CHANGES b/CHANGES index da2554c67..6bba10a61 100644 --- a/CHANGES +++ b/CHANGES @@ -6,6 +6,10 @@ Changelog +Daniel Stenberg (23 Sep 2008) +- Rob Crittenden brought a patch to "add some locking for thread-safety to NSS + implementation". + Daniel Stenberg (22 Sep 2008) - Made the SOCKS code use the new Curl_read_plain() function to fix the bug Markus Moeller reported: http://curl.haxx.se/mail/archive-2008-09/0016.html diff --git a/RELEASE-NOTES b/RELEASE-NOTES index 41928f82c..c9cc9a324 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -25,6 +25,7 @@ This release includes the following bugfixes: o GnuTLS-based multi interface doing HTTPS over proxy failed o recv() failures cause CURLE_RECV_ERROR o SFTP over SOCKS crash fixed + o thread-safety issues addressed for NSS-powered libcurls This release includes the following known bugs: @@ -39,6 +40,6 @@ advice from friends like these: Keith Mok, Yang Tse, Daniel Fandrich, Guenter Knauf, Dmitriy Sergeyev, Linus Nielsen Feltzing, Martin Drasar, Stefan Krause, Dmitry Kurochkin, - Mike Revi, Andres Garcia, Michael Goffioul, Markus Moeller + Mike Revi, Andres Garcia, Michael Goffioul, Markus Moeller, Rob Crittenden Thanks! (and sorry if I forgot to mention someone) diff --git a/lib/nss.c b/lib/nss.c index 0b60486dd..a868fc382 100644 --- a/lib/nss.c +++ b/lib/nss.c @@ -78,7 +78,9 @@ PRFileDesc *PR_ImportTCPSocket(PRInt32 osfd); -int initialized = 0; +PRLock * nss_initlock = NULL; + +volatile int initialized = 0; #define HANDSHAKE_TIMEOUT 30 @@ -837,8 +839,11 @@ static SECStatus SelectClientCert(void *arg, PRFileDesc *sock, */ int Curl_nss_init(void) { - if(!initialized) + /* curl_global_init() is not thread-safe so this test is ok */ + if (nss_initlock == NULL) { PR_Init(PR_USER_THREAD, PR_PRIORITY_NORMAL, 256); + nss_initlock = PR_NewLock(); + } /* We will actually initialize NSS later */ @@ -848,7 +853,17 @@ int Curl_nss_init(void) /* Global cleanup */ void Curl_nss_cleanup(void) { - NSS_Shutdown(); + /* This function isn't required to be threadsafe and this is only done + * as a safety feature. + */ + PR_Lock(nss_initlock); + if (initialized) + NSS_Shutdown(); + PR_Unlock(nss_initlock); + + PR_DestroyLock(nss_initlock); + nss_initlock = NULL; + initialized = 0; } @@ -926,7 +941,8 @@ CURLcode Curl_nss_connect(struct connectdata *conn, int sockindex) return CURLE_OK; /* FIXME. NSS doesn't support multiple databases open at the same time. */ - if(!initialized) { + PR_Lock(nss_initlock); + if(!initialized && !NSS_IsInitialized()) { initialized = 1; certDir = getenv("SSL_DIR"); /* Look in $SSL_DIR */ @@ -950,6 +966,8 @@ CURLcode Curl_nss_connect(struct connectdata *conn, int sockindex) if(rv != SECSuccess) { infof(conn->data, "Unable to initialize NSS database\n"); curlerr = CURLE_SSL_CACERT_BADFILE; + initialized = 0; + PR_Unlock(nss_initlock); goto error; } @@ -972,6 +990,7 @@ CURLcode Curl_nss_connect(struct connectdata *conn, int sockindex) } #endif } + PR_Unlock(nss_initlock); model = PR_NewTCPSocket(); if(!model)