From d64679ee4c125f6df5772007b69a9d7a1b69c32e Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Sun, 30 May 2004 15:39:05 +0000 Subject: [PATCH] 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. --- src/WINNT/afsd/cm_buf.c | 33 ++++++++++++---------- src/WINNT/afsd/cm_cell.c | 69 ++++++++++++++++++++++++++++++++++------------ src/WINNT/afsd/cm_ioctl.c | 50 +++++++++++++++++++++++++++++---- src/WINNT/afsd/cm_scache.c | 2 ++ src/WINNT/afsd/cm_server.c | 35 +++++++++++++++++++++-- src/WINNT/afsd/cm_server.h | 4 +++ src/WINNT/afsd/smb.c | 8 ++++-- src/WINNT/afsd/smb_ioctl.c | 1 + 8 files changed, 158 insertions(+), 44 deletions(-) diff --git a/src/WINNT/afsd/cm_buf.c b/src/WINNT/afsd/cm_buf.c index 6f5a30c..389c7f6 100644 --- 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 --- 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 --- 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 @@ -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 --- 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 --- 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 --- 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 --- 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 --- 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; -- 1.9.4