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.
Differential Revision:
https://phabricator.services.mozilla.com/D47010
--- a/lib/softoken/pkcs11.c
+++ b/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/lib/softoken/pkcs11i.h
+++ b/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/lib/softoken/pkcs11u.c
+++ b/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;
}