summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMartin Thomson <martin.thomson@gmail.com>2018-01-05 11:36:26 +1100
committerMartin Thomson <martin.thomson@gmail.com>2018-01-05 11:36:26 +1100
commit8cb098d179c342d10bdc8a13ace18d648a1ed11e (patch)
treeac0d77e407d3a4f2ac7a6234bd41220e12a96a78
parent8775dd62337b1d4f08896986244fa9de3bf7f0ea (diff)
downloadnss-hg-8cb098d179c342d10bdc8a13ace18d648a1ed11e.tar.gz
Backed out changeset 272dde8958e9
This seems to trigger assertion failures in PR_Unlock across a number of utilities. It seems intermittent and limited to win32 builds. It's also possible that this is a latent bug, but right now the change is making things noticeably worse.
-rw-r--r--lib/dev/devslot.c63
-rw-r--r--lib/dev/devt.h3
-rw-r--r--lib/pk11wrap/dev3hack.c3
3 files changed, 14 insertions, 55 deletions
diff --git a/lib/dev/devslot.c b/lib/dev/devslot.c
index cf5052de1..9f0bd8226 100644
--- a/lib/dev/devslot.c
+++ b/lib/dev/devslot.c
@@ -33,8 +33,6 @@ nssSlot_Destroy(
if (PR_ATOMIC_DECREMENT(&slot->base.refCount) == 0) {
PK11_FreeSlot(slot->pk11slot);
PZ_DestroyLock(slot->base.lock);
- PZ_DestroyCondVar(slot->isPresentCondition);
- PZ_DestroyLock(slot->isPresentLock);
return nssArena_Destroy(slot->base.arena);
}
}
@@ -119,57 +117,35 @@ nssSlot_IsTokenPresent(
nssSession *session;
CK_SLOT_INFO slotInfo;
void *epv;
- PRBool isPresent = PR_FALSE;
-
/* permanent slots are always present unless they're disabled */
if (nssSlot_IsPermanent(slot)) {
return !PK11_IsDisabled(slot->pk11slot);
}
-
/* avoid repeated calls to check token status within set interval */
if (within_token_delay_period(slot)) {
return ((slot->ckFlags & CKF_TOKEN_PRESENT) != 0);
}
- /* First obtain the slot epv before we set up the condition
- * variable, so we can just return if we couldn't get it. */
+ /* First obtain the slot info */
epv = slot->epv;
if (!epv) {
return PR_FALSE;
}
-
- /* set up condition so only one thread is active in this part of the code at a time */
- PZ_Lock(slot->isPresentLock);
- while (slot->inIsPresent) {
- PR_WaitCondVar(slot->isPresentCondition, 0);
- }
- /* if we were one of multiple threads here, the first thread will have
- * given us the answer, no need to make more queries of the token. */
- if (within_token_delay_period(slot)) {
- CK_FLAGS ckFlags = slot->ckFlags;
- PZ_Unlock(slot->isPresentLock);
- return ((ckFlags & CKF_TOKEN_PRESENT) != 0);
- }
- /* this is the winning thread, block all others until we've determined
- * if the token is present and that it needs initialization. */
- slot->inIsPresent = PR_TRUE;
- PZ_Unlock(slot->isPresentLock);
-
nssSlot_EnterMonitor(slot);
ckrv = CKAPI(epv)->C_GetSlotInfo(slot->slotID, &slotInfo);
nssSlot_ExitMonitor(slot);
if (ckrv != CKR_OK) {
slot->token->base.name[0] = 0; /* XXX */
- isPresent = PR_FALSE;
- goto done;
+ slot->lastTokenPing = PR_IntervalNow();
+ return PR_FALSE;
}
slot->ckFlags = slotInfo.flags;
/* check for the presence of the token */
if ((slot->ckFlags & CKF_TOKEN_PRESENT) == 0) {
if (!slot->token) {
/* token was never present */
- isPresent = PR_FALSE;
- goto done;
+ slot->lastTokenPing = PR_IntervalNow();
+ return PR_FALSE;
}
session = nssToken_GetDefaultSession(slot->token);
if (session) {
@@ -191,8 +167,8 @@ nssSlot_IsTokenPresent(
slot->token->base.name[0] = 0; /* XXX */
/* clear the token cache */
nssToken_Remove(slot->token);
- isPresent = PR_FALSE;
- goto done;
+ slot->lastTokenPing = PR_IntervalNow();
+ return PR_FALSE;
}
/* token is present, use the session info to determine if the card
* has been removed and reinserted.
@@ -215,7 +191,8 @@ nssSlot_IsTokenPresent(
nssSession_ExitMonitor(session);
/* token not removed, finished */
if (isPresent) {
- goto done;
+ slot->lastTokenPing = PR_IntervalNow();
+ return PR_TRUE;
}
}
/* the token has been removed, and reinserted, or the slot contains
@@ -226,27 +203,15 @@ nssSlot_IsTokenPresent(
nssToken_Remove(slot->token);
/* token has been removed, need to refresh with new session */
nssrv = nssSlot_Refresh(slot);
- isPresent = PR_TRUE;
if (nssrv != PR_SUCCESS) {
slot->token->base.name[0] = 0; /* XXX */
slot->ckFlags &= ~CKF_TOKEN_PRESENT;
- isPresent = PR_FALSE;
+ /* TODO: insert a barrier here to avoid reordering of the assingments */
+ slot->lastTokenPing = PR_IntervalNow();
+ return PR_FALSE;
}
-done:
- /* Once we've set up the condition variable,
- * Before returning, it's necessary to:
- * 1) Set the lastTokenPing time so that any other threads waiting on this
- * initialization and any future calls within the initialization window
- * return the just-computed status.
- * 2) Indicate we're complete, waking up all other threads that may still
- * be waiting on initialization can progress.
- */
slot->lastTokenPing = PR_IntervalNow();
- PZ_Lock(slot->isPresentLock);
- slot->inIsPresent = PR_FALSE;
- PR_NotifyAllCondVar(slot->isPresentCondition);
- PZ_Unlock(slot->isPresentLock);
- return isPresent;
+ return PR_TRUE;
}
NSS_IMPLEMENT void *
@@ -264,7 +229,7 @@ nssSlot_GetToken(
if (nssSlot_IsTokenPresent(slot)) {
/* Even if a token should be present, check `slot->token` too as it
- * might be gone already. This would happen mostly on shutdown. */
+ * might be gone already. This would happen mostly on shutdown. */
nssSlot_EnterMonitor(slot);
if (slot->token)
rvToken = nssToken_AddRef(slot->token);
diff --git a/lib/dev/devt.h b/lib/dev/devt.h
index e077549ae..db93deb12 100644
--- a/lib/dev/devt.h
+++ b/lib/dev/devt.h
@@ -81,9 +81,6 @@ struct NSSSlotStr {
PZLock *lock;
void *epv;
PK11SlotInfo *pk11slot;
- PZLock *isPresentLock;
- PRCondVar *isPresentCondition;
- PRBool inIsPresent;
};
struct nssSessionStr {
diff --git a/lib/pk11wrap/dev3hack.c b/lib/pk11wrap/dev3hack.c
index 9016eab28..39afd6743 100644
--- a/lib/pk11wrap/dev3hack.c
+++ b/lib/pk11wrap/dev3hack.c
@@ -120,9 +120,6 @@ nssSlot_CreateFromPK11SlotInfo(NSSTrustDomain *td, PK11SlotInfo *nss3slot)
/* Grab the slot name from the PKCS#11 fixed-length buffer */
rvSlot->base.name = nssUTF8_Duplicate(nss3slot->slot_name, td->arena);
rvSlot->lock = (nss3slot->isThreadSafe) ? NULL : nss3slot->sessionLock;
- rvSlot->isPresentLock = PZ_NewLock(nssiLockOther);
- rvSlot->isPresentCondition = PR_NewCondVar(rvSlot->isPresentLock);
- rvSlot->inIsPresent = PR_FALSE;
return rvSlot;
}