Bug 1682863 - Revert nssSlot_IsTokenPresent to 3.58 after ongoing Fx hangs with slow PKCS11 devices. r=bbeurdouche

This patch reverts the `nssSlot_IsTokenPresent` changes made in bug 1663661
and bug 1679290, restoring the version used in NSS 3.58 and earlier. It's not an
actual `hg backout` because the comment in lib/dev/devt.h is worth keeping.
While removing the nested locking did resolve the hang for some (most?) third-party
modules, problems remain with some slower tokens after an even further relaxation
of the locking, which defeats the purpose of addressing the races in the first place.

The crash addressed by these patches was caused by the Intermediate Preloading
Healer in Firefox, which has been disabled. We clearly have insufficient test
coverage for third-party modules, and now that osclientcerts is enabled in Fx
Nightly, any problems caused by these and similar changes is unlikely to be
reported until Fx Beta, well after NSS RTM. I think the best option at this
point is to simply revert NSS.

Differential Revision: https://phabricator.services.mozilla.com/D100344

1 files changed, 6 insertions, 30 deletions

diff --git a/lib/dev/devslot.c b/lib/dev/devslot.c
index 3f4e54e06..5021408bf 100644
--- a/lib/dev/devslot.c
+++ b/lib/dev/devslot.c
@@ -171,12 +171,11 @@ nssSlot_IsTokenPresent(
 
     nssSlot_EnterMonitor(slot);
     ckrv = CKAPI(epv)->C_GetSlotInfo(slot->slotID, &slotInfo);
+    nssSlot_ExitMonitor(slot);
     if (ckrv != CKR_OK) {
-        if (slot->token) {
-            slot->token->base.name[0] = 0; /* XXX */
-        }
+        slot->token->base.name[0] = 0; /* XXX */
         isPresent = PR_FALSE;
-        goto done; /* slot lock held */
+        goto done;
     }
     slot->ckFlags = slotInfo.flags;
     /* check for the presence of the token */
@@ -184,11 +183,10 @@ nssSlot_IsTokenPresent(
         if (!slot->token) {
             /* token was never present */
             isPresent = PR_FALSE;
-            goto done; /* slot lock held */
+            goto done;
         }
         session = nssToken_GetDefaultSession(slot->token);
         if (session) {
-            nssSlot_ExitMonitor(slot);
             nssSession_EnterMonitor(session);
             /* token is not present */
             if (session->handle != CK_INVALID_HANDLE) {
@@ -198,12 +196,6 @@ nssSlot_IsTokenPresent(
                 session->handle = CK_INVALID_HANDLE;
             }
             nssSession_ExitMonitor(session);
-            nssSlot_EnterMonitor(slot);
-            if (!slot->token) {
-                /* Check token presence after re-acquiring lock */
-                isPresent = PR_FALSE;
-                goto done; /* slot lock held */
-            }
         }
         if (slot->token->base.name[0] != 0) {
             /* notify the high-level cache that the token is removed */
@@ -214,23 +206,14 @@ nssSlot_IsTokenPresent(
         /* clear the token cache */
         nssToken_Remove(slot->token);
         isPresent = PR_FALSE;
-        goto done; /* slot lock held */
-    }
-    if (!slot->token) {
-        /* This should not occur, based on the fact that the
-         * below calls will dereference NULL otherwise. */
-        PORT_Assert(0);
-        isPresent = PR_FALSE;
-        goto done; /* slot lock held */
+        goto done;
     }
-
     /* token is present, use the session info to determine if the card
      * has been removed and reinserted.
      */
     session = nssToken_GetDefaultSession(slot->token);
     if (session) {
         PRBool tokenRemoved;
-        nssSlot_ExitMonitor(slot);
         nssSession_EnterMonitor(session);
         if (session->handle != CK_INVALID_HANDLE) {
             CK_SESSION_INFO sessionInfo;
@@ -244,16 +227,10 @@ nssSlot_IsTokenPresent(
         }
         tokenRemoved = (session->handle == CK_INVALID_HANDLE);
         nssSession_ExitMonitor(session);
-        nssSlot_EnterMonitor(slot);
         /* token not removed, finished */
         if (!tokenRemoved) {
             isPresent = PR_TRUE;
-            goto done; /* slot lock held */
-        }
-        if (!slot->token) {
-            /* Check token presence after re-acquiring lock */
-            isPresent = PR_FALSE;
-            goto done; /* slot lock held */
+            goto done;
         }
     }
     /* the token has been removed, and reinserted, or the slot contains
@@ -271,7 +248,6 @@ nssSlot_IsTokenPresent(
         isPresent = PR_FALSE;
     }
 done:
-    nssSlot_ExitMonitor(slot);
     /* Once we've set up the condition variable,
      * Before returning, it's necessary to:
      *  1) Set the lastTokenPingTime so that any other threads waiting on this