Bug 1577822 - land NSS 5619cbbca3db UPGRADE_NSS_RELEASE, r=kjacobs
authorJ.C. Jones <jc@mozilla.com>
Mon, 30 Sep 2019 16:26:14 +0000
changeset 495612 32eea6049fe700ff689a5bd7d11abe249ec0c610
parent 495611 59693d9d9f6036e10fe4656e9c86f52749e8e827
child 495613 73b156a28bfc7f3666a75e5d8e1fd8944ecda658
push id36634
push userapavel@mozilla.com
push dateMon, 30 Sep 2019 21:54:36 +0000
treeherdermozilla-central@4be2a981c307 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskjacobs
bugs1577822, 1508776
milestone71.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1577822 - land NSS 5619cbbca3db UPGRADE_NSS_RELEASE, r=kjacobs 2019-09-27 J.C. Jones <jjones@mozilla.com> * lib/softoken/pkcs11.c, lib/softoken/pkcs11i.h, lib/softoken/pkcs11u.c: Bug 1508776 - Remove unneeded refcounting from SFTKSession r=mt,kjacobs SFTKSession objects are only ever actually destroyed at PK11 session closure, as the session is always the final holder -- and asserting refCount == 1 shows that to be true. Because of that, NSC_CloseSession can just call `sftk_DestroySession` directly and leave `sftk_FreeSession` as a no-op to be removed in the future. [5619cbbca3db] [tip] Differential Revision: https://phabricator.services.mozilla.com/D47631
security/nss/TAG-INFO
security/nss/coreconf/coreconf.dep
security/nss/lib/softoken/pkcs11.c
security/nss/lib/softoken/pkcs11i.h
security/nss/lib/softoken/pkcs11u.c
--- a/security/nss/TAG-INFO
+++ b/security/nss/TAG-INFO
@@ -1,1 +1,1 @@
-be9c48ad76cb
\ No newline at end of file
+5619cbbca3db
\ No newline at end of file
--- a/security/nss/coreconf/coreconf.dep
+++ b/security/nss/coreconf/coreconf.dep
@@ -5,8 +5,9 @@
 
 /*
  * A dummy header file that is a dependency for all the object files.
  * Used to force a full recompilation of NSS in Mozilla's Tinderbox
  * depend builds.  See comments in rules.mk.
  */
 
 #error "Do not include this header file."
+
--- a/security/nss/lib/softoken/pkcs11.c
+++ b/security/nss/lib/softoken/pkcs11.c
@@ -2800,18 +2800,19 @@ sftk_CloseAllSessions(SFTKSlot *slot, PR
                 --slot->sessionCount;
                 SKIP_AFTER_FORK(PZ_Unlock(slot->slotLock));
                 if (session->info.flags & CKF_RW_SESSION) {
                     (void)PR_ATOMIC_DECREMENT(&slot->rwSessionCount);
                 }
             } else {
                 SKIP_AFTER_FORK(PZ_Unlock(lock));
             }
-            if (session)
-                sftk_FreeSession(session);
+            if (session) {
+                sftk_DestroySession(session);
+            }
         } while (session != NULL);
     }
     return CKR_OK;
 }
 
 /*
  * shut down the databases.
  * we get the slot lock (which also protects slot->certDB and slot->keyDB)
@@ -4039,18 +4040,16 @@ NSC_CloseSession(CK_SESSION_HANDLE hSess
     sessionFound = PR_FALSE;
 
     /* lock */
     lock = SFTK_SESSION_LOCK(slot, hSession);
     PZ_Lock(lock);
     if (sftkqueue_is_queued(session, hSession, slot->head, slot->sessHashSize)) {
         sessionFound = PR_TRUE;
         sftkqueue_delete(session, hSession, slot->head, slot->sessHashSize);
-        session->refCount--; /* can't go to zero while we hold the reference */
-        PORT_Assert(session->refCount > 0);
     }
     PZ_Unlock(lock);
 
     if (sessionFound) {
         SFTKDBHandle *handle;
         handle = sftk_getKeyDB(slot);
         PZ_Lock(slot->slotLock);
         if (--slot->sessionCount == 0) {
@@ -4061,19 +4060,20 @@ NSC_CloseSession(CK_SESSION_HANDLE hSess
         }
         PZ_Unlock(slot->slotLock);
         if (handle) {
             sftk_freeDB(handle);
         }
         if (session->info.flags & CKF_RW_SESSION) {
             (void)PR_ATOMIC_DECREMENT(&slot->rwSessionCount);
         }
-    }
-
-    sftk_FreeSession(session);
+        sftk_DestroySession(session);
+        session = NULL;
+    }
+
     return CKR_OK;
 }
 
 /* NSC_CloseAllSessions closes all sessions with a token. */
 CK_RV
 NSC_CloseAllSessions(CK_SLOT_ID slotID)
 {
     SFTKSlot *slot;
--- a/security/nss/lib/softoken/pkcs11i.h
+++ b/security/nss/lib/softoken/pkcs11i.h
@@ -280,17 +280,16 @@ struct SFTKSessionContextStr {
 
 /*
  * Sessions (have objects)
  */
 struct SFTKSessionStr {
     SFTKSession *next;
     SFTKSession *prev;
     CK_SESSION_HANDLE handle;
-    int refCount;
     PZLock *objectLock;
     int objectIDCount;
     CK_SESSION_INFO info;
     CK_NOTIFY notify;
     CK_VOID_PTR appData;
     SFTKSlot *slot;
     SFTKSearchResults *search;
     SFTKSessionContext *enc_context;
@@ -678,16 +677,17 @@ extern void sftk_FreeObjectList(SFTKObje
 extern void sftk_FreeSearch(SFTKSearchResults *search);
 extern CK_RV sftk_handleObject(SFTKObject *object, SFTKSession *session);
 
 extern SFTKSlot *sftk_SlotFromID(CK_SLOT_ID slotID, PRBool all);
 extern SFTKSlot *sftk_SlotFromSessionHandle(CK_SESSION_HANDLE handle);
 extern CK_SLOT_ID sftk_SlotIDFromSessionHandle(CK_SESSION_HANDLE handle);
 extern SFTKSession *sftk_SessionFromHandle(CK_SESSION_HANDLE handle);
 extern void sftk_FreeSession(SFTKSession *session);
+extern void sftk_DestroySession(SFTKSession *session);
 extern SFTKSession *sftk_NewSession(CK_SLOT_ID slotID, CK_NOTIFY notify,
                                     CK_VOID_PTR pApplication, CK_FLAGS flags);
 extern void sftk_update_state(SFTKSlot *slot, SFTKSession *session);
 extern void sftk_update_all_states(SFTKSlot *slot);
 extern void sftk_FreeContext(SFTKSessionContext *context);
 extern void sftk_InitFreeLists(void);
 extern void sftk_CleanupFreeLists(void);
 
--- a/security/nss/lib/softoken/pkcs11u.c
+++ b/security/nss/lib/softoken/pkcs11u.c
@@ -1808,17 +1808,16 @@ sftk_NewSession(CK_SLOT_ID slotID, CK_NO
     if (slot == NULL)
         return NULL;
 
     session = (SFTKSession *)PORT_Alloc(sizeof(SFTKSession));
     if (session == NULL)
         return NULL;
 
     session->next = session->prev = NULL;
-    session->refCount = 1;
     session->enc_context = NULL;
     session->hash_context = NULL;
     session->sign_context = NULL;
     session->search = NULL;
     session->objectIDCount = 1;
     session->objectLock = PZ_NewLock(nssILockObject);
     if (session->objectLock == NULL) {
         PORT_Free(session);
@@ -1832,21 +1831,20 @@ sftk_NewSession(CK_SLOT_ID slotID, CK_NO
     session->info.flags = flags;
     session->info.slotID = slotID;
     session->info.ulDeviceError = 0;
     sftk_update_state(slot, session);
     return session;
 }
 
 /* free all the data associated with a session. */
-static void
+void
 sftk_DestroySession(SFTKSession *session)
 {
     SFTKObjectList *op, *next;
-    PORT_Assert(session->refCount == 0);
 
     /* clean out the attributes */
     /* since no one is referencing us, it's safe to walk the chain
      * without a lock */
     for (op = session->objects[0]; op != NULL; op = next) {
         next = op->next;
         /* paranoia */
         op->next = op->prev = NULL;
@@ -1880,41 +1878,30 @@ sftk_SessionFromHandle(CK_SESSION_HANDLE
     PZLock *lock;
 
     if (!slot)
         return NULL;
     lock = SFTK_SESSION_LOCK(slot, handle);
 
     PZ_Lock(lock);
     sftkqueue_find(session, handle, slot->head, slot->sessHashSize);
-    if (session)
-        session->refCount++;
     PZ_Unlock(lock);
 
     return (session);
 }
 
 /*
- * release a reference to a session handle
+ * release a reference to a session handle. This method of using SFTKSessions
+ * is deprecated, but the pattern should be retained until a future effort
+ * to refactor all SFTKSession users at once is completed.
  */
 void
 sftk_FreeSession(SFTKSession *session)
 {
-    PRBool destroy = PR_FALSE;
-    SFTKSlot *slot = sftk_SlotFromSession(session);
-    PZLock *lock = SFTK_SESSION_LOCK(slot, session->handle);
-
-    PZ_Lock(lock);
-    if (session->refCount == 1)
-        destroy = PR_TRUE;
-    session->refCount--;
-    PZ_Unlock(lock);
-
-    if (destroy)
-        sftk_DestroySession(session);
+    return;
 }
 
 void
 sftk_addHandle(SFTKSearchResults *search, CK_OBJECT_HANDLE handle)
 {
     if (search->handles == NULL) {
         return;
     }