DEVEL15-windows-smb-locking-20080220

LICENSE MIT

minor improvements to smb_rctLock usage.

(cherry picked from commit d48e3d1e7a5a5d065a0046b9115043bed47510a7)

diff --git a/src/WINNT/afsd/cm_user.c b/src/WINNT/afsd/cm_user.c
index 1a0d251e28678bd2a32128dc30a3e91ae2a480db..f5f33a977e6b08183ea4faca3dcad4432628d6c0 100644 (file)
--- a/src/WINNT/afsd/cm_user.c
+++ b/src/WINNT/afsd/cm_user.c
@@ -162,7 +162,7 @@ void cm_CheckTokenCache(time_t now)
     /* 
      * For every vcp, get the user and check his tokens 
      */
-    lock_ObtainWrite(&smb_rctLock);
+    lock_ObtainRead(&smb_rctLock);
     for (vcp=smb_allVCsp; vcp; vcp=vcp->nextp) {
         for (usersp=vcp->usersp; usersp; usersp=usersp->nextp) {
             if (usersp->unp) {
@@ -194,7 +194,7 @@ void cm_CheckTokenCache(time_t now)
             }
         }
     }
-    lock_ReleaseWrite(&smb_rctLock);
+    lock_ReleaseRead(&smb_rctLock);
 }
 
 #ifdef USE_ROOT_TOKENS

diff --git a/src/WINNT/afsd/smb.c b/src/WINNT/afsd/smb.c
index f99d7ba2b11ea666f0c828c2de878ccc9becc0ca..9951987dac347f4bd927ccdc07f474d0fc971817 100644 (file)
--- a/src/WINNT/afsd/smb.c
+++ b/src/WINNT/afsd/smb.c
@@ -903,7 +903,7 @@ smb_vc_t *smb_FindVC(unsigned short lsn, int flags, int lana)
 {
     smb_vc_t *vcp;
 
-       lock_ObtainWrite(&smb_globalLock);      /* for numVCs */
+    lock_ObtainWrite(&smb_globalLock); /* for numVCs */
     lock_ObtainWrite(&smb_rctLock);
     for (vcp = smb_allVCsp; vcp; vcp=vcp->nextp) {
        if (vcp->magic != SMB_VC_MAGIC)
@@ -979,7 +979,7 @@ smb_vc_t *smb_FindVC(unsigned short lsn, int flags, int lana)
         }
     }
     lock_ReleaseWrite(&smb_rctLock);
-       lock_ReleaseWrite(&smb_globalLock);
+    lock_ReleaseWrite(&smb_globalLock);
     return vcp;
 }
 
@@ -1004,35 +1004,35 @@ void smb_ReleaseVCInternal(smb_vc_t *vcp)
     vcp->refCount--;
 
     if (vcp->refCount == 0) {
-      if (vcp->flags & SMB_VCFLAG_ALREADYDEAD) {
-       /* remove VCP from smb_deadVCsp */
-       for (vcpp = &smb_deadVCsp; *vcpp; vcpp = &((*vcpp)->nextp)) {
-         if (*vcpp == vcp) {
-           *vcpp = vcp->nextp;
-           break;
-         }
-       } 
-       lock_FinalizeMutex(&vcp->mx);
-       memset(vcp,0,sizeof(smb_vc_t));
-       free(vcp);
-      } else {
-       for (avcp = smb_allVCsp; avcp; avcp = avcp->nextp) {
-         if (avcp == vcp)
-           break;
-       }
-       osi_Log3(smb_logp,"VCP not dead and %sin smb_allVCsp vcp %x ref %d",
-                avcp?"not ":"",vcp, vcp->refCount);
+        if (vcp->flags & SMB_VCFLAG_ALREADYDEAD) {
+            /* remove VCP from smb_deadVCsp */
+            for (vcpp = &smb_deadVCsp; *vcpp; vcpp = &((*vcpp)->nextp)) {
+                if (*vcpp == vcp) {
+                    *vcpp = vcp->nextp;
+                    break;
+                }
+            } 
+            lock_FinalizeMutex(&vcp->mx);
+            memset(vcp,0,sizeof(smb_vc_t));
+            free(vcp);
+        } else {
+            for (avcp = smb_allVCsp; avcp; avcp = avcp->nextp) {
+                if (avcp == vcp)
+                    break;
+            }
+            osi_Log3(smb_logp,"VCP not dead and %sin smb_allVCsp vcp %x ref %d",
+                      avcp?"not ":"",vcp, vcp->refCount);
 #ifdef DEBUG
-       GenerateMiniDump(NULL);
+            GenerateMiniDump(NULL);
 #endif
-       /* This is a wrong.  However, I suspect that there is an undercount
-        * and I don't want to release 1.4.1 in a state that will allow
-        * smb_vc_t objects to be deallocated while still in the
-        * smb_allVCsp list.  The list is supposed to keep a reference
-        * to the smb_vc_t.  Put it back.
-        */
-       vcp->refCount++;
-      }
+            /* This is a wrong.  However, I suspect that there is an undercount
+             * and I don't want to release 1.4.1 in a state that will allow
+             * smb_vc_t objects to be deallocated while still in the
+             * smb_allVCsp list.  The list is supposed to keep a reference
+             * to the smb_vc_t.  Put it back.
+             */
+            vcp->refCount++;
+        }
     }
 }
 
@@ -1131,11 +1131,7 @@ void smb_CleanupDeadVC(smb_vc_t *vcp)
        osi_Log2(smb_logp, "  Cleanup TID %d (tidp=0x%x)", tid, tidpIter);
 
        smb_HoldTIDNoLock(tidpIter);
-       lock_ReleaseWrite(&smb_rctLock);
-
-       smb_ReleaseTID(tidpIter);
-
-       lock_ObtainWrite(&smb_rctLock);
+       smb_ReleaseTID(tidpIter, TRUE);
        tidpNext = vcp->tidsp;
     }
 
@@ -1172,9 +1168,7 @@ smb_tid_t *smb_FindTID(smb_vc_t *vcp, unsigned short tid, int flags)
     for (tidp = vcp->tidsp; tidp; tidp = tidp->nextp) {
        if (tidp->refCount == 0 && tidp->delete) {
            tidp->refCount++;
-           lock_ReleaseWrite(&smb_rctLock);
-           smb_ReleaseTID(tidp);
-           lock_ObtainWrite(&smb_rctLock);
+           smb_ReleaseTID(tidp, TRUE);
            goto retry;
        }
 
@@ -1203,14 +1197,15 @@ void smb_HoldTIDNoLock(smb_tid_t *tidp)
     tidp->refCount++;
 }
 
-void smb_ReleaseTID(smb_tid_t *tidp)
+void smb_ReleaseTID(smb_tid_t *tidp, afs_uint32 locked)
 {
     smb_tid_t *tp;
     smb_tid_t **ltpp;
     cm_user_t *userp;
 
     userp = NULL;
-    lock_ObtainWrite(&smb_rctLock);
+    if (!locked)
+        lock_ObtainWrite(&smb_rctLock);
     osi_assertx(tidp->refCount-- > 0, "smb_tid_t refCount 0");
     if (tidp->refCount == 0 && (tidp->delete)) {
         ltpp = &tidp->vcp->tidsp;
@@ -1226,7 +1221,8 @@ void smb_ReleaseTID(smb_tid_t *tidp)
         smb_ReleaseVCNoLock(tidp->vcp);
         tidp->vcp = NULL;
     }
-    lock_ReleaseWrite(&smb_rctLock);
+    if (!locked)
+        lock_ReleaseWrite(&smb_rctLock);
     if (userp)
         cm_ReleaseUser(userp);
 }              
@@ -1338,10 +1334,8 @@ void smb_ReleaseUsername(smb_username_t *unp)
        free(unp);
     }          
     lock_ReleaseWrite(&smb_rctLock);
-
-    if (userp) {
+    if (userp)
         cm_ReleaseUser(userp);
-    }  
 }      
 
 void smb_HoldUIDNoLock(smb_user_t *uidp)
@@ -1437,7 +1431,7 @@ long smb_LookupTIDPath(smb_vc_t *vcp, unsigned short tid, char ** treepath)
             /* tidp->pathname would be NULL, but that's fine */
         }
         *treepath = tidp->pathname;
-       smb_ReleaseTID(tidp);
+       smb_ReleaseTID(tidp, FALSE);
     }
     return code;
 }
@@ -3214,6 +3208,7 @@ long smb_ReceiveCoreReadRaw(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp
             lock_ReleaseMutex(&smb_RawBufLock);
         }
 
+        lock_ReleaseMutex(&fidp->mx);
         smb_ReleaseFID(fidp);
         return rc;
     }
@@ -3627,11 +3622,8 @@ void smb_Daemon(void *parmp)
                free(unp->name);
                free(unp->machine);
                free(unp);
-               if (userp) {
-                   lock_ReleaseWrite(&smb_rctLock);
+               if (userp)
                    cm_ReleaseUser(userp);
-                   lock_ObtainWrite(&smb_rctLock);
-               }
            } else {
                unpp = &(*unpp)->nextp;
            }
@@ -3832,14 +3824,14 @@ long smb_ReceiveCoreTreeConnect(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *
     if (uidp)
         smb_ReleaseUID(uidp);
     if (!shareFound) {
-        smb_ReleaseTID(tidp);
+        smb_ReleaseTID(tidp, FALSE);
         return CM_ERROR_BADSHARENAME;
     }
     lock_ObtainMutex(&tidp->mx);
     tidp->userp = userp;
     tidp->pathname = sharePath;
     lock_ReleaseMutex(&tidp->mx);
-    smb_ReleaseTID(tidp);
+    smb_ReleaseTID(tidp, FALSE);
 
     smb_SetSMBParm(rsp, 0, SMB_PACKETSIZE);
     smb_SetSMBParm(rsp, 1, newTid);
@@ -5062,8 +5054,8 @@ long smb_ReceiveCoreTreeDisconnect(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_
     if (tidp) {
        lock_ObtainWrite(&smb_rctLock);
         tidp->delete = 1;
+        smb_ReleaseTID(tidp, TRUE);
         lock_ReleaseWrite(&smb_rctLock);
-        smb_ReleaseTID(tidp);
     }
 
     return 0;
@@ -9749,9 +9741,7 @@ void smb_Shutdown(void)
             if (tidp->userp) {
                 cm_user_t *userp = tidp->userp;
                 tidp->userp = NULL;
-                lock_ReleaseWrite(&smb_rctLock);
                 cm_ReleaseUser(userp);
-                lock_ObtainWrite(&smb_rctLock);
             }
         }
     }

diff --git a/src/WINNT/afsd/smb.h b/src/WINNT/afsd/smb.h
index 05565d70b808aa9bcc9fd22e07e46448011e33a7..dba2eadc6447786d1c3f8815233b29d4a8febd76 100644 (file)
--- a/src/WINNT/afsd/smb.h
+++ b/src/WINNT/afsd/smb.h
@@ -563,7 +563,7 @@ extern smb_tid_t *smb_FindTID(smb_vc_t *vcp, unsigned short tid, int flags);
 
 extern void smb_HoldTIDNoLock(smb_tid_t *tidp);
 
-extern void smb_ReleaseTID(smb_tid_t *tidp);
+extern void smb_ReleaseTID(smb_tid_t *tidp, afs_uint32 locked);
 
 extern smb_user_t *smb_FindUID(smb_vc_t *vcp, unsigned short uid, int flags);
 

diff --git a/src/WINNT/afsd/smb3.c b/src/WINNT/afsd/smb3.c
index f85ba5892118839453f525eb601c0377640ffd8e..95715deb9d2f9fcd73070225dc45f183e313017c 100644 (file)
--- a/src/WINNT/afsd/smb3.c
+++ b/src/WINNT/afsd/smb3.c
@@ -1063,7 +1063,7 @@ long smb_ReceiveV3TreeConnectX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *o
        if (!shareFound) {
            if (uidp)
                smb_ReleaseUID(uidp);
-            smb_ReleaseTID(tidp);
+            smb_ReleaseTID(tidp, FALSE);
             return CM_ERROR_BADSHARENAME;
        }
 
@@ -1100,7 +1100,7 @@ long smb_ReceiveV3TreeConnectX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *o
     if (ipc) 
         tidp->flags |= SMB_TIDFLAG_IPC;
     lock_ReleaseMutex(&tidp->mx);
-    smb_ReleaseTID(tidp);
+    smb_ReleaseTID(tidp, FALSE);
 
     ((smb_t *)outp)->tid = newTid;
     ((smb_t *)inp)->tid = newTid;
@@ -4663,6 +4663,7 @@ long smb_T2SearchDirSingle(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t *op
     smb_FreeTran2Packet(outp);
     if (dep)
         free(dep);
+    if (scp)
     cm_ReleaseSCache(scp);
     cm_ReleaseSCache(targetscp);
     cm_ReleaseUser(userp);