Start adding threading checks to nss_util.

crypto/nss_util.cc

Index: crypto/nss_util.cc
diff --git a/crypto/nss_util.cc b/crypto/nss_util.cc
index 04080ed3afc41245e4fce341f88cf9280e20bf2d..ecc131cd58c65b855578b3f3ec5a3a092cf843c5 100644
--- a/crypto/nss_util.cc
+++ b/crypto/nss_util.cc
@@ -24,6 +24,7 @@
 #include <vector>
 
 #include "base/debug/alias.h"
+#include "base/debug/stack_trace.h"
 #include "base/environment.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
@@ -34,6 +35,7 @@
 #include "base/metrics/histogram.h"
 #include "base/native_library.h"
 #include "base/strings/stringprintf.h"
+#include "base/threading/non_thread_safe.h"
 #include "base/threading/thread_restrictions.h"
 #include "build/build_config.h"
 
@@ -213,10 +215,12 @@ void CrashOnNSSInitFailure() {
   LOG(FATAL) << "nss_error=" << nss_error << ", os_error=" << os_error;
 }
 
-class NSSInitSingleton {
+class NSSInitSingleton : public base::NonThreadSafe {
  public:
 #if defined(OS_CHROMEOS)
   void OpenPersistentNSSDB() {
+    DCHECK(CalledOnValidThread());
+
     if (!chromeos_user_logged_in_) {
       // GetDefaultConfigDirectory causes us to do blocking IO on UI thread.
       // Temporarily allow it until we fix http://crbug.com/70119
@@ -233,6 +237,8 @@ class NSSInitSingleton {
   }
 
   void EnableTPMTokenForNSS() {
+    DCHECK(CalledOnValidThread());
+
     // If this gets set, then we'll use the TPM for certs with
     // private keys, otherwise we'll fall back to the software
     // implementation.
@@ -242,6 +248,8 @@ class NSSInitSingleton {
   bool InitializeTPMToken(const std::string& token_name,
                           int token_slot_id,
                           const std::string& user_pin) {
+    DCHECK(CalledOnValidThread());
+
     // If EnableTPMTokenForNSS hasn't been called, return false.
     if (!tpm_token_enabled_for_nss_)
       return false;
@@ -281,6 +289,12 @@ class NSSInitSingleton {
   }
 
   void GetTPMTokenInfo(std::string* token_name, std::string* user_pin) {
+    // TODO(mattm): Change to DCHECK when callers have been fixed.
+    if (!CalledOnValidThread()) {
+      DVLOG(1) << "Called on wrong thread.\n"
+               << base::debug::StackTrace().ToString();
+    }
+
     if (!tpm_token_enabled_for_nss_) {
       LOG(ERROR) << "GetTPMTokenInfo called before TPM Token is ready.";
       return;
@@ -292,6 +306,12 @@ class NSSInitSingleton {
   }
 
   bool IsTPMTokenReady() {
+    // TODO(mattm): Change to DCHECK when callers have been fixed.
+    if (!CalledOnValidThread()) {
+      DVLOG(1) << "Called on wrong thread.\n"
+               << base::debug::StackTrace().ToString();
+    }
+
     return tpm_slot_ != NULL;
   }
 
@@ -299,6 +319,8 @@ class NSSInitSingleton {
   // id as an int. This should be safe since this is only used with chaps, which
   // we also control.
   PK11SlotInfo* GetTPMSlotForId(CK_SLOT_ID slot_id) {
+    DCHECK(CalledOnValidThread());
+
     if (!chaps_module_)
       return NULL;
 
@@ -316,6 +338,8 @@ class NSSInitSingleton {
 
 
   bool OpenTestNSSDB() {
+    DCHECK(CalledOnValidThread());
+
     if (test_slot_)
       return true;
     if (!g_test_nss_db_dir.Get().CreateUniqueTempDir())
@@ -325,6 +349,8 @@ class NSSInitSingleton {
   }
 
   void CloseTestNSSDB() {
+    DCHECK(CalledOnValidThread());
+
     if (!test_slot_)
       return;
     SECStatus status = SECMOD_CloseUserDB(test_slot_);
@@ -336,6 +362,12 @@ class NSSInitSingleton {
   }
 
   PK11SlotInfo* GetPublicNSSKeySlot() {
+    // TODO(mattm): Change to DCHECK when callers have been fixed.
+    if (!CalledOnValidThread()) {
+      DVLOG(1) << "Called on wrong thread.\n"
+               << base::debug::StackTrace().ToString();
+    }
+
     if (test_slot_)
       return PK11_ReferenceSlot(test_slot_);
     if (software_slot_)
@@ -344,6 +376,12 @@ class NSSInitSingleton {
   }
 
   PK11SlotInfo* GetPrivateNSSKeySlot() {
+    // TODO(mattm): Change to DCHECK when callers have been fixed.
+    if (!CalledOnValidThread()) {
+      DVLOG(1) << "Called on wrong thread.\n"
+               << base::debug::StackTrace().ToString();
+    }
+
     if (test_slot_)
       return PK11_ReferenceSlot(test_slot_);
 
@@ -487,6 +525,10 @@ class NSSInitSingleton {
                            base::TimeDelta::FromMilliseconds(10),
                            base::TimeDelta::FromHours(1),
                            50);
+
+    // It's safe to construct on any thread, since LazyInstance will prevent any
+    // other threads from accessing until the constructor is done.
+    DetachFromThread();

[wtc, 2013/11/11 22:10:15]

If this can be called at the beginning of this function, we should do that. This
information should be more visible, and it will also protect us from early
returns.

[mattm, 2013/11/12 00:41:34]

Done.

   }
 
   // NOTE(willchan): We don't actually execute this code since we leak NSS to