[NSS] Bug 1508776 - Remove unneeded refcounting from SFTKSession

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.
This commit is contained in:
J.C. Jones 2019-12-06 15:36:44 +01:00 • committed by Roy Tam
commit 0916deba59
3 changed files with 11 additions and 23 deletions

View file

@ -2674,8 +2674,9 @@ sftk_CloseAllSessions(SFTKSlot *slot, PRBool logout)
} else { } else {
SKIP_AFTER_FORK(PZ_Unlock(lock)); SKIP_AFTER_FORK(PZ_Unlock(lock));
} }
if (session) if (session) {
sftk_FreeSession(session); sftk_DestroySession(session);
}
} while (session != NULL); } while (session != NULL);
} }
return CKR_OK; return CKR_OK;
@ -3910,8 +3911,6 @@ NSC_CloseSession(CK_SESSION_HANDLE hSession)
if (sftkqueue_is_queued(session, hSession, slot->head, slot->sessHashSize)) { if (sftkqueue_is_queued(session, hSession, slot->head, slot->sessHashSize)) {
sessionFound = PR_TRUE; sessionFound = PR_TRUE;
sftkqueue_delete(session, hSession, slot->head, slot->sessHashSize); 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); PZ_Unlock(lock);
@ -3932,6 +3931,8 @@ NSC_CloseSession(CK_SESSION_HANDLE hSession)
if (session->info.flags & CKF_RW_SESSION) { if (session->info.flags & CKF_RW_SESSION) {
(void)PR_ATOMIC_DECREMENT(&slot->rwSessionCount); (void)PR_ATOMIC_DECREMENT(&slot->rwSessionCount);
} }
sftk_DestroySession(session);
session = NULL;
} }
sftk_FreeSession(session); sftk_FreeSession(session);

View file

@ -281,7 +281,6 @@ struct SFTKSessionStr {
SFTKSession *next; SFTKSession *next;
SFTKSession *prev; SFTKSession *prev;
CK_SESSION_HANDLE handle; CK_SESSION_HANDLE handle;
int refCount;
PZLock *objectLock; PZLock *objectLock;
int objectIDCount; int objectIDCount;
CK_SESSION_INFO info; CK_SESSION_INFO info;
@ -670,6 +669,7 @@ extern SFTKSlot *sftk_SlotFromSessionHandle(CK_SESSION_HANDLE handle);
extern CK_SLOT_ID sftk_SlotIDFromSessionHandle(CK_SESSION_HANDLE handle); extern CK_SLOT_ID sftk_SlotIDFromSessionHandle(CK_SESSION_HANDLE handle);
extern SFTKSession *sftk_SessionFromHandle(CK_SESSION_HANDLE handle); extern SFTKSession *sftk_SessionFromHandle(CK_SESSION_HANDLE handle);
extern void sftk_FreeSession(SFTKSession *session); extern void sftk_FreeSession(SFTKSession *session);
extern void sftk_DestroySession(SFTKSession *session);
extern SFTKSession *sftk_NewSession(CK_SLOT_ID slotID, CK_NOTIFY notify, extern SFTKSession *sftk_NewSession(CK_SLOT_ID slotID, CK_NOTIFY notify,
CK_VOID_PTR pApplication, CK_FLAGS flags); CK_VOID_PTR pApplication, CK_FLAGS flags);
extern void sftk_update_state(SFTKSlot *slot, SFTKSession *session); extern void sftk_update_state(SFTKSlot *slot, SFTKSession *session);

View file

@ -1772,7 +1772,6 @@ sftk_NewSession(CK_SLOT_ID slotID, CK_NOTIFY notify, CK_VOID_PTR pApplication,
return NULL; return NULL;
session->next = session->prev = NULL; session->next = session->prev = NULL;
session->refCount = 1;
session->enc_context = NULL; session->enc_context = NULL;
session->hash_context = NULL; session->hash_context = NULL;
session->sign_context = NULL; session->sign_context = NULL;
@ -1796,11 +1795,10 @@ sftk_NewSession(CK_SLOT_ID slotID, CK_NOTIFY notify, CK_VOID_PTR pApplication,
} }
/* free all the data associated with a session. */ /* free all the data associated with a session. */
static void void
sftk_DestroySession(SFTKSession *session) sftk_DestroySession(SFTKSession *session)
{ {
SFTKObjectList *op, *next; SFTKObjectList *op, *next;
PORT_Assert(session->refCount == 0);
/* clean out the attributes */ /* clean out the attributes */
/* since no one is referencing us, it's safe to walk the chain /* since no one is referencing us, it's safe to walk the chain
@ -1844,31 +1842,20 @@ sftk_SessionFromHandle(CK_SESSION_HANDLE handle)
PZ_Lock(lock); PZ_Lock(lock);
sftkqueue_find(session, handle, slot->head, slot->sessHashSize); sftkqueue_find(session, handle, slot->head, slot->sessHashSize);
if (session)
session->refCount++;
PZ_Unlock(lock); PZ_Unlock(lock);
return (session); 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 void
sftk_FreeSession(SFTKSession *session) sftk_FreeSession(SFTKSession *session)
{ {
PRBool destroy = PR_FALSE; return;
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);
} }
void void