dns-and-server-ref-counts-20040530

   * Add debug info to test whether CM_BUF_WAITING or CM_SCACHE_WAITING
     are ever set more than once at a time

   * Fix the management of lists of cm_cell_t structures when using
     DNS to lookup cell information.  The previous code would fail to
     reuse the same cellID for a cell if DNS was used more than once
     for a given cell name.  When the ttl expired, a single cm_cell_t
     could be inserted into the cm_allCellsp list more than once
     producing a loop.  In addition, the vlServerp list belonging to
     the cell was not freed resulting in improper refCounting of the
     servers.

   * Add DNS support to cm_IoctlNewCell() which previous only examined
     the CellServDB file

   * Add cm_FreeServer() function and call it from cm_FreeServerList()
     to properly garbage collect cm_server_t objects

   * Add numVCs variable to smb.c to track the number of smb_vc_t
     objects created and use it to initialize the vcID field which
     previously was set to 0 in all objects resulting in FindByID
     collisions.

diff --git a/src/WINNT/afsd/cm_buf.c b/src/WINNT/afsd/cm_buf.c
index 6f5a30c..389c7f6 100644 (file)
--- a/src/WINNT/afsd/cm_buf.c
+++ b/src/WINNT/afsd/cm_buf.c
@@ -502,24 +502,27 @@ void buf_WaitIO(cm_buf_t *bp)
                if (!(bp->flags & (CM_BUF_READING | CM_BUF_WRITING)))
                        break;
                
-                /* otherwise I/O is happening, but some other thread is waiting for
-                 * the I/O already.  Wait for that guy to figure out what happened,
-                 * and then check again.
-                 */
-                bp->flags |= CM_BUF_WAITING;
-                osi_SleepM((long) bp, &bp->mx);
-                lock_ObtainMutex(&bp->mx);
+        /* otherwise I/O is happening, but some other thread is waiting for
+         * the I/O already.  Wait for that guy to figure out what happened,
+         * and then check again.
+         */
+        if ( bp->flags & CM_BUF_WAITING ) 
+            osi_Log1(buf_logp, "buf_WaitIO CM_BUF_WAITING already set for 0x%x", bp);
+
+        bp->flags |= CM_BUF_WAITING;
+        osi_SleepM((long) bp, &bp->mx);
+        lock_ObtainMutex(&bp->mx);
                osi_Log1(buf_logp, "buf_WaitIO conflict wait done for 0x%x", bp);
-        }
+    }
         
-        /* if we get here, the IO is done, but we may have to wakeup people waiting for
-         * the I/O to complete.  Do so.
-         */
-        if (bp->flags & CM_BUF_WAITING) {
+    /* if we get here, the IO is done, but we may have to wakeup people waiting for
+     * the I/O to complete.  Do so.
+     */
+    if (bp->flags & CM_BUF_WAITING) {
                bp->flags &= ~CM_BUF_WAITING;
-                osi_Wakeup((long) bp);
-        }
-        osi_Log1(buf_logp, "WaitIO finished wait for bp 0x%x", (long) bp);
+        osi_Wakeup((long) bp);
+    }
+    osi_Log1(buf_logp, "WaitIO finished wait for bp 0x%x", (long) bp);
 }
 
 /* code to drop reference count while holding buf_globalLock */

diff --git a/src/WINNT/afsd/cm_cell.c b/src/WINNT/afsd/cm_cell.c
index c2e942a..3de3aa8 100644 (file)
--- a/src/WINNT/afsd/cm_cell.c
+++ b/src/WINNT/afsd/cm_cell.c
@@ -34,7 +34,7 @@ long cm_AddCellProc(void *rockp, struct sockaddr_in *addrp, char *namep)
 {
        cm_server_t *tsp;
        cm_serverRef_t *tsrp;
-        cm_cell_t *cellp;
+    cm_cell_t *cellp;
         
        cellp = rockp;
 
@@ -82,8 +82,18 @@ cm_cell_t *cm_GetCell_Gen(char *namep, char *newnamep, long flags)
          || (cp && (cp->flags & CM_CELLFLAG_DNS) && (time(0) > cp->timeout))
 #endif
          ) {
-               if (!cp) cp = malloc(sizeof(*cp));
-        memset(cp, 0, sizeof(*cp));
+        int dns_expired = 0;
+               if (!cp) {
+            cp = malloc(sizeof(*cp));
+            memset(cp, 0, sizeof(*cp));
+        } 
+        else {
+            dns_expired = 1;
+            /* must empty cp->vlServersp */
+            cm_FreeServerList(&cp->vlServersp);
+            cp->vlServersp = NULL;
+        }
+
         code = cm_SearchCellFile(namep, fullname, cm_AddCellProc, cp);
                if (code) {
             afsi_log("in cm_GetCell_gen cm_SearchCellFile(%s) returns code= %d fullname= %s", 
@@ -92,9 +102,31 @@ cm_cell_t *cm_GetCell_Gen(char *namep, char *newnamep, long flags)
 #ifdef AFS_AFSDB_ENV
             if (cm_dnsEnabled /*&& cm_DomainValid(namep)*/) {
                 code = cm_SearchCellByDNS(namep, fullname, &ttl, cm_AddCellProc, cp);
-                if ( code )
+                if ( code ) {
                     afsi_log("in cm_GetCell_gen cm_SearchCellByDNS(%s) returns code= %d fullname= %s", 
                              namep, code, fullname);
+                    if (dns_expired) {
+                        if ( cm_allCellsp == cp )
+                            cm_allCellsp = cp->nextp;
+                        else {
+                            cm_cell_t *tcp;
+
+                            for(tcp = cm_allCellsp; tcp->nextp; tcp=tcp->nextp) {
+                                if ( tcp->nextp == cp ) {
+                                    tcp->nextp = cp->nextp;
+                                    break;
+                                }
+                            }
+                        }
+                    
+                        lock_FinalizeMutex(&cp->mx);
+                        free(cp->namep);
+                    }
+                }
+                else {   /* got cell from DNS */
+                    cp->flags |= CM_CELLFLAG_DNS;
+                    cp->timeout = time(0) + ttl;
+                }
             }
 #endif
             if (code) {
@@ -102,17 +134,20 @@ cm_cell_t *cm_GetCell_Gen(char *namep, char *newnamep, long flags)
                 cp = NULL;
                 goto done;
             }
-#ifdef AFS_AFSDB_ENV
-            else {   /* got cell from DNS */
-                cp->flags |= CM_CELLFLAG_DNS;
-                cp->timeout = time(0) + ttl;
-            }
-#endif
                }
 
                /* randomise among those vlservers having the same rank*/ 
                cm_RandomizeServer(&cp->vlServersp);
 
+#ifdef AFS_AFSDB_ENV
+        if (dns_expired) {
+            /* we want to preserve the full name and mutex.
+             * also, cp is already in the cm_allCellsp list
+             */
+            goto done;
+        }
+#endif /* AFS_AFSDB_ENV */
+
         /* otherwise we found the cell, and so we're nearly done */
         lock_InitializeMutex(&cp->mx, "cm_cell_t mutex");
 
@@ -130,9 +165,9 @@ cm_cell_t *cm_GetCell_Gen(char *namep, char *newnamep, long flags)
   done:
     /* fullname is not valid if cp == NULL */
        if (cp && newnamep)
-         strcpy(newnamep, fullname);
+        strcpy(newnamep, fullname);
        lock_ReleaseWrite(&cm_cellLock);
-        return cp;
+    return cp;
 }
 
 cm_cell_t *cm_FindCellByID(long cellID)
@@ -143,8 +178,9 @@ cm_cell_t *cm_FindCellByID(long cellID)
 
        lock_ObtainWrite(&cm_cellLock);
        for(cp = cm_allCellsp; cp; cp=cp->nextp) {
-               if (cellID == cp->cellID) break;
-        }
+               if (cellID == cp->cellID) 
+            break;
+    }
 
 #ifdef AFS_AFSDB_ENV
        /* if it's from DNS, see if it has expired */
@@ -163,8 +199,7 @@ cm_cell_t *cm_FindCellByID(long cellID)
 #endif /* AFS_AFSDB_ENV */
 
        lock_ReleaseWrite(&cm_cellLock);        
-       
-        return cp;
+    return cp;
 }
 
 void cm_InitCell(void)
@@ -177,7 +212,7 @@ void cm_InitCell(void)
                osi_EndOnce(&once);
         }
 }
-void cm_ChangeRankCellVLServer(cm_server_t       *tsp)
+void cm_ChangeRankCellVLServer(cm_server_t *tsp)
 {
        cm_cell_t *cp;
        int code;

diff --git a/src/WINNT/afsd/cm_ioctl.c b/src/WINNT/afsd/cm_ioctl.c
index 57099b3..187b9cf 100644 (file)
--- a/src/WINNT/afsd/cm_ioctl.c
+++ b/src/WINNT/afsd/cm_ioctl.c
@@ -28,6 +28,7 @@
 #include "afsd_init.h"
 
 #include "smb.h"
+#include "cm_server.h"
 
 #ifndef DJGPP
 #include <rx/rxkad.h>
@@ -938,18 +939,55 @@ long cm_IoctlNewCell(struct smb_ioctl *ioctlp, struct cm_user *userp)
      * cell list will be cm_CellLock and cm_ServerLock will be held for write.
     */  
   
-    cm_cell_t *tcellp;
+    cm_cell_t *cp;
+    cm_cell_t *tcp;
   
     cm_SkipIoctlPath(ioctlp);
     lock_ObtainWrite(&cm_cellLock);
   
-    for(tcellp = cm_allCellsp; tcellp; tcellp=tcellp->nextp) 
+    for(cp = cm_allCellsp; cp; cp=cp->nextp) 
     {
+        long code;
+      top:
         /* delete all previous server lists - cm_FreeServerList will ask for write on cm_ServerLock*/
-        cm_FreeServerList(&tcellp->vlServersp);
-        tcellp->vlServersp = NULL;
-        cm_SearchCellFile(tcellp->namep, tcellp->namep, cm_AddCellProc, tcellp);
-        cm_RandomizeServer(&tcellp->vlServersp);
+        cm_FreeServerList(&cp->vlServersp);
+        cp->vlServersp = NULL;
+        code = cm_SearchCellFile(cp->namep, cp->namep, cm_AddCellProc, cp);
+               if (code) {
+#ifdef AFS_AFSDB_ENV
+            if (cm_dnsEnabled) {
+                int ttl;
+                code = cm_SearchCellByDNS(cp->namep, cp->namep, &ttl, cm_AddCellProc, cp);
+                if ( code ) {
+                    if ( cm_allCellsp == cp )
+                        cm_allCellsp = cp->nextp;
+                    else {
+                        for(tcp = cm_allCellsp; tcp->nextp; tcp=tcp->nextp) {
+                            if ( tcp->nextp == cp ) {
+                                tcp->nextp = cp->nextp;
+                                break;
+                            }
+                        }
+                    }   
+                    
+                    lock_FinalizeMutex(&cp->mx);
+                    free(cp->namep);
+                }
+                else {   /* got cell from DNS */
+                    cp->flags |= CM_CELLFLAG_DNS;
+                    cp->timeout = time(0) + ttl;
+                }
+            }
+#endif /* AFS_AFSDB_ENV */
+        }
+        if (code) {
+            tcp = cp;
+            cp = cp->nextp;
+            free(tcp);
+            goto top;
+        }
+        else
+            cm_RandomizeServer(&cp->vlServersp);
     }
     
     lock_ReleaseWrite(&cm_cellLock);

diff --git a/src/WINNT/afsd/cm_scache.c b/src/WINNT/afsd/cm_scache.c
index 587e381..2b8eaf7 100644 (file)
--- a/src/WINNT/afsd/cm_scache.c
+++ b/src/WINNT/afsd/cm_scache.c
@@ -644,6 +644,8 @@ sleep:
 
                /* wait here, then try again */
                osi_Log1(afsd_logp, "CM SyncOp sleeping scp %x", (long) scp);
+        if ( scp->flags & CM_SCACHEFLAG_WAITING ) 
+            osi_Log1(afsd_logp, "CM SyncOp CM_SCACHEFLAG_WAITING already set for 0x%x", scp);
                scp->flags |= CM_SCACHEFLAG_WAITING;
                if (bufLocked) lock_ReleaseMutex(&bufp->mx);
         osi_SleepM((long) &scp->flags, &scp->mx);

diff --git a/src/WINNT/afsd/cm_server.c b/src/WINNT/afsd/cm_server.c
index 71cfc64..77707e1 100644 (file)
--- a/src/WINNT/afsd/cm_server.c
+++ b/src/WINNT/afsd/cm_server.c
@@ -390,6 +390,34 @@ void cm_RandomizeServer(cm_serverRef_t** list)
     lock_ReleaseWrite(&cm_serverLock);
 }
 
+void cm_FreeServer(cm_server_t* server)
+{
+    lock_ObtainWrite(&cm_serverLock);
+    if (--(server->refCount) == 0)
+    {
+        /* we need to check to ensure that all of the connections
+         * for this server have a 0 refCount; otherwise, they will
+         * not be garbage collected 
+         */
+        cm_GCConnections(&server);  /* connsp */
+
+        lock_FinalizeMutex(&server->mx);
+        if ( cm_allServersp == server )
+            cm_allServersp = server->allNextp;
+        else {
+            cm_server_t *tsp;
+
+            for(tsp = cm_allServersp; tsp->allNextp; tsp=tsp->allNextp) {
+                if ( tsp->allNextp == server ) {
+                    tsp->allNextp = server->allNextp;
+                    break;
+                }
+            }
+        }
+    }
+    lock_ReleaseWrite(&cm_serverLock);
+}
+
 void cm_FreeServerList(cm_serverRef_t** list)
 {
     cm_serverRef_t  *current = *list;
@@ -399,9 +427,10 @@ void cm_FreeServerList(cm_serverRef_t** list)
 
     while (current)
     {
-          next = current->next;
-          free(current);
-          current = next;
+        next = current->next;
+        cm_FreeServer(current->server);
+        free(current);
+        current = next;
     }
   
     lock_ReleaseWrite(&cm_serverLock);

diff --git a/src/WINNT/afsd/cm_server.h b/src/WINNT/afsd/cm_server.h
index c0b357f..114a378 100644 (file)
--- a/src/WINNT/afsd/cm_server.h
+++ b/src/WINNT/afsd/cm_server.h
@@ -93,4 +93,8 @@ extern long cm_ChangeRankServer(cm_serverRef_t** list, cm_server_t* server);
 
 extern void cm_RandomizeServer(cm_serverRef_t** list); 
 
+extern void cm_FreeServer(cm_server_t* server);
+
+extern void cm_FreeServerList(cm_serverRef_t** list);
+
 #endif /*  __CM_SERVER_H_ENV__ */

diff --git a/src/WINNT/afsd/smb.c b/src/WINNT/afsd/smb.c
index c6b2d6e..a585f67 100644 (file)
--- a/src/WINNT/afsd/smb.c
+++ b/src/WINNT/afsd/smb.c
@@ -82,7 +82,7 @@ smb_ncb_t *smb_ncbFreeListp;
 
 int smb_NumServerThreads;
 
-int numNCBs, numSessions;
+int numNCBs, numSessions, numVCs;
 
 int smb_maxVCPerServer;
 int smb_maxMpxRequests;
@@ -728,6 +728,7 @@ smb_vc_t *smb_FindVC(unsigned short lsn, int flags, int lana)
        if (!vcp && (flags & SMB_FLAG_CREATE)) {
                vcp = malloc(sizeof(*vcp));
                memset(vcp, 0, sizeof(*vcp));
+        vcp->vcID = numVCs++;
                vcp->refCount = 1;
                vcp->tidCounter = 1;
                vcp->fidCounter = 1;
@@ -6690,8 +6691,8 @@ void smb_Listener(void *parmp)
          * we run out.
          */
 
-        osi_assert(i < Sessionmax);
-        osi_assert(numNCBs < NCBmax);
+        osi_assert(i < Sessionmax - 1);
+        osi_assert(numNCBs < NCBmax - 1);   /* if we pass this test we can allocate one more */
 
                LSNs[i] = ncbp->ncb_lsn;
                lanas[i] = ncbp->ncb_lana_num;
@@ -7028,6 +7029,7 @@ void smb_Init(osi_log_t *logp, char *snamep, int useV3, int LANadapt,
     smb_NetbiosInit();
 
        /* Initialize listener and server structures */
+    numVCs = 0;
        memset(dead_sessions, 0, sizeof(dead_sessions));
     sprintf(eventName, "SessionEvents[0]");
        SessionEvents[0] = thrd_CreateEvent(NULL, FALSE, FALSE, eventName);

diff --git a/src/WINNT/afsd/smb_ioctl.c b/src/WINNT/afsd/smb_ioctl.c
index b8391d3..db5cbf0 100644 (file)
--- a/src/WINNT/afsd/smb_ioctl.c
+++ b/src/WINNT/afsd/smb_ioctl.c
@@ -276,6 +276,7 @@ long smb_IoctlV3Read(smb_fid_t *fidp, smb_vc_t *vcp, smb_packet_t *inp, smb_pack
         count = smb_GetSMBParm(inp, 5);
        
        userp = smb_GetUser(vcp, inp);
+       osi_assert(userp != 0);
 
        {
                smb_user_t *uidp;