windows-smb-lock-timeouts-20080228

LICENSE MIT

Attempts to open files which are already write-locked by another
client took forever to return a lock not granted error.  This
was because cm_Analyze() would retry the lock request for up to
the RDRtimeout in response to the EAGAIN error.  The problem
was that cm_IntSetLock() was not setting the CM_REQ_NORETRY flag.

While examining this issue, discovered two other things:

(1) the infinite wait logic on lock request processing was broken
(2) the cancel outstanding lock request logic wasn't implemented
(3) cm_Analyze() would put the thread to sleep even when retries
    were not permitted.

Also removed a number of compile time warnings.

diff --git a/src/WINNT/afsd/cm.h b/src/WINNT/afsd/cm.h
index 4273392..3b3161a 100644 (file)
--- a/src/WINNT/afsd/cm.h
+++ b/src/WINNT/afsd/cm.h
@@ -90,6 +90,8 @@
 #define CM_ERROR_BPLUS_NOMATCH          (CM_ERROR_BASE+55)
 #define CM_ERROR_EAS_NOT_SUPPORTED      (CM_ERROR_BASE+56)
 #define CM_ERROR_RANGE_NOT_LOCKED       (CM_ERROR_BASE+57)
+#define CM_ERROR_NOSUCHDEVICE           (CM_ERROR_BASE+58)
+#define CM_ERROR_LOCK_NOT_GRANTED       (CM_ERROR_BASE+59)
 
 /* Used by cm_FollowMountPoint and cm_GetVolumeByName */
 #define RWVOL  0

diff --git a/src/WINNT/afsd/cm_conn.c b/src/WINNT/afsd/cm_conn.c
index 7ff818d..585c052 100644 (file)
--- a/src/WINNT/afsd/cm_conn.c
+++ b/src/WINNT/afsd/cm_conn.c
@@ -202,7 +202,10 @@ cm_Analyze(cm_conn_t *connp, cm_user_t *userp, cm_req_t *reqp,
     timeUsed = (GetTickCount() - reqp->startTime) / 1000;
            
     /* leave 5 seconds margin for sleep */
-    timeLeft = HardDeadtimeout - timeUsed;
+    if (reqp->flags & CM_REQ_NORETRY)
+        timeLeft = 0;
+    else
+        timeLeft = HardDeadtimeout - timeUsed;
 
     /* get a pointer to the cell */
     if (errorCode) {
@@ -221,7 +224,7 @@ cm_Analyze(cm_conn_t *connp, cm_user_t *userp, cm_req_t *reqp,
     }
 
     if (errorCode == CM_ERROR_TIMEDOUT) {
-        if (timeLeft > 5 ) {
+        if ( timeLeft > 5 ) {
             thrd_Sleep(3000);
             cm_CheckServers(CM_FLAG_CHECKDOWNSERVERS, cellp);
             retry = 1;
@@ -231,7 +234,7 @@ cm_Analyze(cm_conn_t *connp, cm_user_t *userp, cm_req_t *reqp,
     else if (errorCode == UAEWOULDBLOCK || errorCode == EWOULDBLOCK ||
               errorCode == UAEAGAIN || errorCode == EAGAIN) {
        osi_Log0(afsd_logp, "cm_Analyze passed EWOULDBLOCK or EAGAIN.");
-        if (timeLeft > 5 ) {
+        if ( timeLeft > 5 ) {
             thrd_Sleep(1000);
             retry = 1;
         }

diff --git a/src/WINNT/afsd/cm_dcache.h b/src/WINNT/afsd/cm_dcache.h
index 8cde66d..63f4264 100644 (file)
--- a/src/WINNT/afsd/cm_dcache.h
+++ b/src/WINNT/afsd/cm_dcache.h
@@ -57,4 +57,7 @@ extern long cm_ValidateDCache(void);
 
 extern long cm_ShutdownDCache(void);
 
+extern long cm_BufWrite(void *vscp, osi_hyper_t *offsetp, long length, long flags,
+                 cm_user_t *userp, cm_req_t *reqp);
+
 #endif /*  __CM_DCACHE_ENV__ */

diff --git a/src/WINNT/afsd/cm_vnodeops.c b/src/WINNT/afsd/cm_vnodeops.c
index 8c13ee1..6d77ede 100644 (file)
--- a/src/WINNT/afsd/cm_vnodeops.c
+++ b/src/WINNT/afsd/cm_vnodeops.c
@@ -3963,6 +3963,7 @@ long cm_IntSetLock(cm_scache_t * scp, cm_user_t * userp, int lockType,
     cm_conn_t * connp;
     struct rx_connection * callp;
     AFSVolSync volSync;
+    afs_uint32 reqflags = reqp->flags;
 
     tfid.Volume = scp->fid.volume;
     tfid.Vnode = scp->fid.vnode;
@@ -3971,6 +3972,7 @@ long cm_IntSetLock(cm_scache_t * scp, cm_user_t * userp, int lockType,
 
     osi_Log2(afsd_logp, "CALL SetLock scp 0x%p for lock %d", scp, lockType);
 
+    reqp->flags |= CM_REQ_NORETRY;
     lock_ReleaseMutex(&scp->mx);
 
     do {
@@ -3994,7 +3996,7 @@ long cm_IntSetLock(cm_scache_t * scp, cm_user_t * userp, int lockType,
     }
 
     lock_ObtainMutex(&scp->mx);
-
+    reqp->flags = reqflags;
     return code;
 }
 

diff --git a/src/WINNT/afsd/smb.c b/src/WINNT/afsd/smb.c
index a44ee14..f5bd160 100644 (file)
--- a/src/WINNT/afsd/smb.c
+++ b/src/WINNT/afsd/smb.c
@@ -78,9 +78,9 @@ smb_dispatch_t smb_dispatchTable[SMB_NOPCODES];
 smb_packet_t *smb_packetFreeListp;
 smb_ncb_t *smb_ncbFreeListp;
 
-int smb_NumServerThreads;
+afs_uint32 smb_NumServerThreads;
 
-int numNCBs, numSessions, numVCs;
+afs_uint32 numNCBs, numSessions, numVCs;
 
 int smb_maxVCPerServer;
 int smb_maxMpxRequests;
@@ -214,7 +214,7 @@ void smb_UpdateServerPriority()
        time_t now = osi_Time();
 
        /* Give one priority boost for each 15 seconds */
-       SetThreadPriority(GetCurrentThread(), (now - *tp) / 15);
+       SetThreadPriority(GetCurrentThread(), (int)((now - *tp) / 15));
     }
 }
 
@@ -1664,7 +1664,7 @@ int smb_FindShare(smb_vc_t *vcp, smb_user_t *uidp, char *shareName,
         snprintf(pathstr, sizeof(pathstr)/sizeof(char),
                  "/" CM_PREFIX_VOL "%s", shareName);
         pathstr[sizeof(pathstr)/sizeof(char) - 1] = '\0';
-        len = strlen(pathstr) + 1;
+        len = (DWORD)(strlen(pathstr) + 1);
 
         *pathNamep = malloc(len);
         if (*pathNamep) {
@@ -2641,7 +2641,12 @@ void smb_MapNTError(long code, unsigned long *NTStatusp)
     else if (code == CM_ERROR_RANGE_NOT_LOCKED) {
        NTStatus = 0xC000007EL; /* Range Not Locked */
     } 
-    else {
+    else if (code == CM_ERROR_NOSUCHDEVICE) {
+        NTStatus = 0xC000000EL; /* No Such Device */
+    }
+    else if (code == CM_ERROR_LOCK_NOT_GRANTED) {
+        NTStatus = 0xC0000055L; /* Lock Not Granted */
+    } else {
         NTStatus = 0xC0982001L;        /* SMB non-specific error */
     }
 
@@ -3396,6 +3401,11 @@ void smb_WaitingLocksDaemon()
                 if (wl->state == SMB_WAITINGLOCKSTATE_DONE)
                     continue;
 
+                if (wl->state == SMB_WAITINGLOCKSTATE_CANCELLED) {
+                    code = CM_ERROR_LOCK_NOT_GRANTED;
+                    break;
+                }
+
                 osi_assertx(wl->state != SMB_WAITINGLOCKSTATE_ERROR, "!SMB_WAITINGLOCKSTATE_ERROR");
                 
                 /* wl->state is either _DONE or _WAITING.  _ERROR
@@ -3414,8 +3424,8 @@ void smb_WaitingLocksDaemon()
             if (code == CM_ERROR_WOULDBLOCK) {
 
                 /* no progress */
-                if (wlRequest->timeRemaining != 0xffffffff
-                     && (wlRequest->timeRemaining -= 1000) < 0)
+                if (wlRequest->msTimeout != 0xffffffff
+                     && ((osi_Time() - wlRequest->start_t) * 1000 > wlRequest->msTimeout))
                     goto endWait;
 
                 continue;
@@ -3439,9 +3449,10 @@ void smb_WaitingLocksDaemon()
 
                 for (wl = wlRequest->locks; wl; wl = wlNext) {
                     wlNext = (smb_waitingLock_t *) osi_QNext(&wl->q);
-                    
-                    cm_Unlock(scp, wlRequest->lockType, wl->LOffset, 
-                              wl->LLength, wl->key, NULL, &req);
+                
+                    if (wl->state == SMB_WAITINGLOCKSTATE_DONE)
+                        cm_Unlock(scp, wlRequest->lockType, wl->LOffset, 
+                                  wl->LLength, wl->key, NULL, &req);
 
                     osi_QRemove((osi_queue_t **) &wlRequest->locks, &wl->q);
 
@@ -6019,7 +6030,7 @@ long smb_ReceiveCoreClose(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp)
 /*
  * smb_ReadData -- common code for Read, Read And X, and Raw Read
  */
-long smb_ReadData(smb_fid_t *fidp, osi_hyper_t *offsetp, long count, char *op,
+long smb_ReadData(smb_fid_t *fidp, osi_hyper_t *offsetp, afs_uint32 count, char *op,
        cm_user_t *userp, long *readp)
 {
     osi_hyper_t offset;
@@ -6030,7 +6041,8 @@ long smb_ReadData(smb_fid_t *fidp, osi_hyper_t *offsetp, long count, char *op,
     osi_hyper_t thyper;
     osi_hyper_t lastByte;
     osi_hyper_t bufferOffset;
-    long bufIndex, nbytes;
+    long bufIndex;
+    afs_uint32 nbytes;
     int chunk;
     int sequential = (fidp->flags & SMB_FID_SEQUENTIAL);
     cm_req_t req;
@@ -6170,7 +6182,7 @@ long smb_ReadData(smb_fid_t *fidp, osi_hyper_t *offsetp, long count, char *op,
 /*
  * smb_WriteData -- common code for Write and Raw Write
  */
-long smb_WriteData(smb_fid_t *fidp, osi_hyper_t *offsetp, long count, char *op,
+long smb_WriteData(smb_fid_t *fidp, osi_hyper_t *offsetp, afs_uint32 count, char *op,
        cm_user_t *userp, long *writtenp)
 {
     osi_hyper_t offset = *offsetp;
@@ -8018,7 +8030,7 @@ void InitNCBslot(int idx)
 {
     struct smb_packet *bufp;
     EVENT_HANDLE retHandle;
-    int i;
+    afs_uint32 i;
     char eventName[MAX_PATH];
 
     osi_assertx( idx < (sizeof(NCBs) / sizeof(NCBs[0])), "invalid index" );
@@ -8050,7 +8062,7 @@ void smb_Listener(void *parmp)
     long code = 0;
     long len;
     long i;
-    int  session, thread;
+    afs_uint32  session, thread;
     smb_vc_t *vcp = NULL;
     int flags = 0;
     char rname[NCBNAMSZ+1];
@@ -9120,7 +9132,7 @@ void smb_Shutdown(void)
 {
     NCB *ncbp;
     long code = 0;
-    int i;
+    afs_uint32 i;
     smb_vc_t *vcp;
 
     /*fprintf(stderr, "Entering smb_Shutdown\n");*/

diff --git a/src/WINNT/afsd/smb.h b/src/WINNT/afsd/smb.h
index 6a80a93..86ec4a9 100644 (file)
--- a/src/WINNT/afsd/smb.h
+++ b/src/WINNT/afsd/smb.h
@@ -482,9 +482,10 @@ typedef struct smb_waitingLock {
     int              state;
 } smb_waitingLock_t;
 
-#define SMB_WAITINGLOCKSTATE_WAITING 0
-#define SMB_WAITINGLOCKSTATE_DONE    1
-#define SMB_WAITINGLOCKSTATE_ERROR   2
+#define SMB_WAITINGLOCKSTATE_WAITING   0
+#define SMB_WAITINGLOCKSTATE_DONE      1
+#define SMB_WAITINGLOCKSTATE_ERROR     2
+#define SMB_WAITINGLOCKSTATE_CANCELLED 3
 
 /* waiting lock request */
 typedef struct smb_waitingLockRequest {
@@ -494,7 +495,8 @@ typedef struct smb_waitingLockRequest {
     smb_packet_t *inp;
     smb_packet_t *outp;
     int           lockType;
-    time_t        timeRemaining;
+    time_t        start_t;              /* osi_Time */
+    afs_uint32    msTimeout;            /* msecs, 0xFFFFFFFF = wait forever */
     smb_waitingLock_t * locks;
 } smb_waitingLockRequest_t;
 
@@ -713,10 +715,10 @@ extern unsigned char *smb_ParseVblBlock(unsigned char *inp, char **chainpp, int
 
 extern int smb_SUser(cm_user_t *userp);
 
-long smb_WriteData(smb_fid_t *fidp, osi_hyper_t *offsetp, long count, char *op,
+long smb_WriteData(smb_fid_t *fidp, osi_hyper_t *offsetp, afs_uint32 count, char *op,
        cm_user_t *userp, long *writtenp);
 
-extern long smb_ReadData(smb_fid_t *fidp, osi_hyper_t *offsetp, long count,
+extern long smb_ReadData(smb_fid_t *fidp, osi_hyper_t *offsetp, afs_uint32 count,
        char *op, cm_user_t *userp, long *readp);
 
 extern long smb_Rename(smb_vc_t *vcp, smb_packet_t *inp, char *oldPathp, char *newPathp, int attrs);

diff --git a/src/WINNT/afsd/smb3.c b/src/WINNT/afsd/smb3.c
index 4dfa3c3..6aa394b 100644 (file)
--- a/src/WINNT/afsd/smb3.c
+++ b/src/WINNT/afsd/smb3.c
@@ -73,10 +73,10 @@ afs_uint32 smb_IsExecutableFileName(const char *name)
     if ( smb_ExecutableExtensions == NULL || name == NULL)
         return 0;
 
-    len = strlen(name);
+    len = (int)strlen(name);
 
     for ( i=0; smb_ExecutableExtensions[i]; i++) {
-        j = len - strlen(smb_ExecutableExtensions[i]);
+        j = len - (int)strlen(smb_ExecutableExtensions[i]);
         if (_stricmp(smb_ExecutableExtensions[i], &name[j]) == 0)
             return 1;
     }
@@ -2883,7 +2883,7 @@ long smb_ReceiveTran2QPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t
         goto done;
     }
     else if (infoLevel == SMB_QUERY_FILE_NAME_INFO) {
-       len = strlen(lastComp);
+       len = (unsigned int)strlen(lastComp);
        qpi.u.QPfileNameInfo.fileNameLength = (len + 1) * 2;
         mbstowcs((unsigned short *)qpi.u.QPfileNameInfo.fileName, lastComp, len + 1);
 
@@ -2961,7 +2961,7 @@ long smb_ReceiveTran2QPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t
        qpi.u.QPfileAllInfo.currentByteOffset.LowPart = 0;
        qpi.u.QPfileAllInfo.mode = 0;
        qpi.u.QPfileAllInfo.alignmentRequirement = 0;
-       len = strlen(lastComp);
+       len = (unsigned int)strlen(lastComp);
        qpi.u.QPfileAllInfo.fileNameLength = (len + 1) * 2;
         mbstowcs((unsigned short *)qpi.u.QPfileAllInfo.fileName, lastComp, len + 1);
     }
@@ -3627,8 +3627,8 @@ smb_ReceiveTran2GetDFSReferral(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t
     osi_Log2(smb_logp,"ReceiveTran2GetDfsReferral [%d][%s]", 
              maxReferralLevel, osi_LogSaveString(smb_logp, requestFileName));
 
-    nbnLen = strlen(cm_NetbiosName);
-    reqLen = strlen(requestFileName);
+    nbnLen = (int)strlen(cm_NetbiosName);
+    reqLen = (int)strlen(requestFileName);
 
     if (reqLen > nbnLen + 2 && requestFileName[0] == '\\' &&
         !_strnicmp(cm_NetbiosName,&requestFileName[1],nbnLen) &&
@@ -3705,10 +3705,10 @@ smb_ReceiveTran2GetDFSReferral(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t
                 /* scp should now be the DfsLink we are looking for */
                 if (scp) {
                     /* figure out how much of the input path was used */
-                    reqLen = nbnLen+2 + strlen(pathName) + 1 + strlen(lastComponent);
+                    reqLen = (int)(nbnLen+2 + strlen(pathName) + 1 + strlen(lastComponent));
 
                     strcpy(referralPath, &scp->mountPointStringp[strlen("msdfs:")]);
-                    refLen = strlen(referralPath);
+                    refLen = (int)strlen(referralPath);
                     found = 1;
                 }
             } else {
@@ -3792,7 +3792,7 @@ smb_ReceiveTran2GetDFSReferral(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t
     return 0;
 #else /* DFS_SUPPORT */
     osi_Log0(smb_logp,"ReceiveTran2GetDfsReferral - NOT_SUPPORTED"); 
-    return CM_ERROR_BADOP;
+    return CM_ERROR_NOSUCHDEVICE;
 #endif /* DFS_SUPPORT */
 }
 
@@ -4530,7 +4530,7 @@ long smb_T2SearchDirSingle(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t *op
         ohbytes += 4;           /* EASIZE */
 
     /* add header to name & term. null */
-    onbytes = strlen(maskp);
+    onbytes = (int)strlen(maskp);
     orbytes = ohbytes + onbytes + 1;
 
     /* now, we round up the record to a 4 byte alignment, and we make
@@ -5783,7 +5783,7 @@ long smb_ReceiveV3LockingX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp)
     cm_scache_t *scp;
     unsigned char LockType;
     unsigned short NumberOfUnlocks, NumberOfLocks;
-    long Timeout;
+    afs_uint32 Timeout;
     char *op;
     char *op_locks;
     LARGE_INTEGER LOffset, LLength;
@@ -5850,18 +5850,9 @@ long smb_ReceiveV3LockingX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp)
         LockType |= LOCKING_ANDX_SHARED_LOCK;
     }
 
-    if ((LockType & LOCKING_ANDX_CANCEL_LOCK) ||
-        (LockType & LOCKING_ANDX_CHANGE_LOCKTYPE)) {
-
-        /* We don't support these requests.  Apparently, we can safely
-           not deal with them too. */
-        osi_Log1(smb_logp, "smb_ReceiveV3Locking received unsupported request [%s]",
-                 ((LockType & LOCKING_ANDX_CANCEL_LOCK)?
-                  "LOCKING_ANDX_CANCEL_LOCK":
-                  "LOCKING_ANDX_CHANGE_LOCKTYPE")); 
-        /* No need to call osi_LogSaveString since these are string
-           constants.*/
-
+    if (LockType & LOCKING_ANDX_CHANGE_LOCKTYPE) {
+        /* AFS does not support atomic changes of lock types from read or write and vice-versa */
+        osi_Log0(smb_logp, "smb_ReceiveV3Locking received unsupported request [LOCKING_ANDX_CHANGE_LOCKTYPE]"); 
         code = CM_ERROR_BADOP;
         goto done;
 
@@ -5869,6 +5860,35 @@ long smb_ReceiveV3LockingX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp)
 
     op = smb_GetSMBData(inp, NULL);
 
+    if (LockType & LOCKING_ANDX_CANCEL_LOCK) {
+        /* Cancel outstanding lock requests */
+        smb_waitingLock_t * wl;
+
+        for (i=0; i<NumberOfLocks; i++) {
+            smb_GetLockParams(LockType, &op, &pid, &LOffset, &LLength);
+
+            key = cm_GenerateKey(vcp->vcID, pid, fidp->fid);
+
+            lock_ObtainWrite(&smb_globalLock);
+            for (wlRequest = smb_allWaitingLocks; wlRequest; wlRequest = (smb_waitingLockRequest_t *) osi_QNext(&wlRequest->q))
+            {
+                for (wl = wlRequest->locks; wl; wl = (smb_waitingLock_t *) osi_QNext(&wl->q)) {
+                    if (wl->key == key && LargeIntegerEqualTo(wl->LOffset, LOffset) && 
+                        LargeIntegerEqualTo(wl->LLength, LLength)) {
+                        wl->state = SMB_WAITINGLOCKSTATE_CANCELLED;
+                        goto found_lock_request;
+                    }
+                }
+            }
+          found_lock_request:
+            lock_ReleaseWrite(&smb_globalLock);
+        }
+        code = 0;
+        smb_SetSMBDataLength(outp, 0);
+        goto done;
+    }
+
+
     for (i=0; i<NumberOfUnlocks; i++) {
         smb_GetLockParams(LockType, &op, &pid, &LOffset, &LLength);
 
@@ -5919,7 +5939,8 @@ long smb_ReceiveV3LockingX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp)
                 wlRequest->inp = smb_CopyPacket(inp);
                 wlRequest->outp = smb_CopyPacket(outp);
                 wlRequest->lockType = LockType;
-                wlRequest->timeRemaining = Timeout;
+                wlRequest->msTimeout = Timeout;
+                wlRequest->start_t = osi_Time();
                 wlRequest->locks = NULL;
 
                 /* The waiting lock request needs to have enough
@@ -6315,8 +6336,6 @@ long smb_ReceiveV3WriteX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp)
         written = 0;
     }
 
- done_writing:
-    
     /* slots 0 and 1 are reserved for request chaining and will be
        filled in when we return. */
     smb_SetSMBParm(outp, 2, total_written);
@@ -7199,7 +7218,7 @@ long smb_ReceiveNTCreateX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp)
         LLength.LowPart = SMB_FID_QLOCK_LENGTH;
 
         /* If we are not opening the file for writing, then we don't
-           try to get an exclusive lock.  Noone else should be able to
+           try to get an exclusive lock.  No one else should be able to
            get an exclusive lock on the file anyway, although someone
            else can get a shared lock. */
         if ((fidflags & SMB_FID_SHARE_READ) ||
@@ -8254,6 +8273,12 @@ long smb_ReceiveNTTransact(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp)
        break;
     case 6:
         return smb_ReceiveNTTranQuerySecurityDesc(vcp, inp, outp);
+    case 7:
+        osi_Log0(smb_logp, "SMB NT Transact Query Quota - not implemented");
+        break;
+    case 8:
+        osi_Log0(smb_logp, "SMB NT Transact Set Quota - not implemented");
+        break;
     }
     return CM_ERROR_INVAL;
 }