From 477cca589d05afccec461ad5bfd61aa08ab6ac58 Mon Sep 17 00:00:00 2001
From: David Keeler <dkeeler@mozilla.com>
Date: Thu, 11 May 2017 13:22:52 -0700
Subject: [PATCH] Bug 1363932 - reduce locking overhead in
 sftk_searchObjectList r=franziskus

Summary:
Before this patch, sftk_searchObjectList would acquire and release the
SFTKSlot's objectLock once per bucket in the SFTKSlot's sessObjHashTable. This
patch reduces the locking overhead by acquiring the lock once and then iterating
over the entire table.

This patch also removes the unused PRBool tokenOnly in NSC_FindObjectsInit (the
only caller of sftk_searchObjectList). (Changeset b8f289456399 removed the code
that modified tokenOnly. Unfortunately there doesn't appear to be a bug
associated with that changeset, so we can't easily determine if that was
intended. In any case, it's been this way for a decade, so if anything was
broken by this, no one has noticed yet.)

Reviewers: franziskus

Reviewed By: franziskus

Differential Revision: https://nss-review.dev.mozaws.net/D316

--HG--
extra : amend_source : b9454fd1fcd5fc587c41e7c39eb81d031730168e
---
 lib/softoken/pkcs11.c  | 14 +++++---------
 lib/softoken/pkcs11u.c |  6 ++----
 2 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/lib/softoken/pkcs11.c b/lib/softoken/pkcs11.c
index 1e091589cf..a594fd501b 100644
--- a/lib/softoken/pkcs11.c
+++ b/lib/softoken/pkcs11.c
@@ -4763,7 +4763,7 @@ sftk_pruneSearch(CK_ATTRIBUTE *pTemplate, CK_ULONG ulCount,
 static CK_RV
 sftk_searchTokenList(SFTKSlot *slot, SFTKSearchResults *search,
                      CK_ATTRIBUTE *pTemplate, CK_ULONG ulCount,
-                     PRBool *tokenOnly, PRBool isLoggedIn)
+                     PRBool isLoggedIn)
 {
     CK_RV crv = CKR_OK;
     CK_RV crv2;
@@ -4798,7 +4798,6 @@ NSC_FindObjectsInit(CK_SESSION_HANDLE hSession,
     SFTKSearchResults *search = NULL, *freeSearch = NULL;
     SFTKSession *session = NULL;
     SFTKSlot *slot = sftk_SlotFromSessionHandle(hSession);
-    PRBool tokenOnly = PR_FALSE;
     CK_RV crv = CKR_OK;
     PRBool isLoggedIn;
 
@@ -4829,18 +4828,15 @@ NSC_FindObjectsInit(CK_SESSION_HANDLE hSession,
     search->array_size = NSC_SEARCH_BLOCK_SIZE;
     isLoggedIn = (PRBool)((!slot->needLogin) || slot->isLoggedIn);
 
-    crv = sftk_searchTokenList(slot, search, pTemplate, ulCount, &tokenOnly,
-                               isLoggedIn);
+    crv = sftk_searchTokenList(slot, search, pTemplate, ulCount, isLoggedIn);
     if (crv != CKR_OK) {
         goto loser;
     }
 
     /* build list of found objects in the session */
-    if (!tokenOnly) {
-        crv = sftk_searchObjectList(search, slot->sessObjHashTable,
-                                    slot->sessObjHashSize, slot->objectLock,
-                                    pTemplate, ulCount, isLoggedIn);
-    }
+    crv = sftk_searchObjectList(search, slot->sessObjHashTable,
+                                slot->sessObjHashSize, slot->objectLock,
+                                pTemplate, ulCount, isLoggedIn);
     if (crv != CKR_OK) {
         goto loser;
     }
diff --git a/lib/softoken/pkcs11u.c b/lib/softoken/pkcs11u.c
index a5694ee382..c51211b6c8 100644
--- a/lib/softoken/pkcs11u.c
+++ b/lib/softoken/pkcs11u.c
@@ -1649,10 +1649,8 @@ sftk_searchObjectList(SFTKSearchResults *search, SFTKObject **head,
     SFTKObject *object;
     CK_RV crv = CKR_OK;
 
+    PZ_Lock(lock);
     for (i = 0; i < size; i++) {
-        /* We need to hold the lock to copy a consistant version of
-         * the linked list. */
-        PZ_Lock(lock);
         for (object = head[i]; object != NULL; object = object->next) {
             if (sftk_objectMatch(object, theTemplate, count)) {
                 /* don't return objects that aren't yet visible */
@@ -1661,8 +1659,8 @@ sftk_searchObjectList(SFTKSearchResults *search, SFTKObject **head,
                 sftk_addHandle(search, object->handle);
             }
         }
-        PZ_Unlock(lock);
     }
+    PZ_Unlock(lock);
     return crv;
 }