4th set of fixes for apache-fastcgi-2.0b1
Sonya Rikhtverchik (rikhtver@OpenMarket.com)
Tue, 02 Sep 1997 11:25:14 -0400
Message-Id: <199709021525.LAA00515@u4-138.openmarket.com>
To: fastcgi-developers@OpenMarket.com
Subject: 4th set of fixes for apache-fastcgi-2.0b1
Date: Tue, 02 Sep 1997 11:25:14 -0400
From: Sonya Rikhtverchik <rikhtver@OpenMarket.com>
Date: Wed, 27 Aug 1997 18:25:14 -0400 (EDT)
Message-Id: <SAA22271.199708272225@bastille.va.pubnix.com>
From: "David J. MacKenzie" <djm@va.pubnix.com>
To: fastcgi-developers@openmarket.com
cc: djm@va.pubnix.com
Subject: 4th set of fixes for apache-fastcgi-2.0b1
This patch fixes the problems noted below.
Problems fixed by this patch, which should be applied after the others
I've sent:
1. Remove the dead lock file and socket when cleaning up after a
server whose last child has been killed, so the FastCGIHandler isn't
fooled into thinking there is still a process serving that app.
2. Plug a memory leak when doing a blocking kill.
3. Make file locking robust in the presence of signals.
4. Rename some badly named variables.
5. Fixes typos in many comments.
- --- mod_fastcgi.c 1997/08/26 17:16:51 1.4
+++ mod_fastcgi.c 1997/08/27 22:04:36
@@ -818,7 +818,7 @@
goto GET_IPC_ERROR_EXIT;
}
#ifndef __EMX__
- - /* OS/2 dosen't support changing ownership. */
+ /* OS/2 doesn't support changing ownership. */
chown(DStringValue(&ipcAddrPtr->bindPath), uid, gid);
#endif
@@ -2354,7 +2354,8 @@
/*
* MakeLockFileName
*
- - * Returns the lock file name for application execName.
+ * Returns the lock file name for dynamic application execName.
+ * Allocates memory for it.
*/
char *
@@ -2504,6 +2505,9 @@
{
FcgiProcessInfo *processInfoPtr;
int i;
+ char *lockFileName;
+ OS_IpcAddr *ipcAddrPtr;
+ char *fname;
/*
* Free up process/connection info.
@@ -2516,6 +2520,22 @@
}
OS_FreeIpcAddr(processInfoPtr->ipcAddr);
}
+
+ /*
+ * Remove the dead lock file and socket.
+ */
+ lockFileName = MakeLockFileName(DStringValue(&serverInfoPtr->execPath));
+ unlink(lockFileName);
+ Free(lockFileName);
+
+ ipcAddrPtr = (OS_IpcAddr *)serverInfoPtr->ipcAddr;
+ fname = MakeSocketName(DStringValue(&serverInfoPtr->execPath),
+ NULL, -1, &ipcAddrPtr->bindPath, 1);
+ /* Remove extraneous digit from end. XXX why is this necessary? */
+ fname[strlen(fname)-1] = '\0';
+ unlink(fname);
+ Free(fname);
+
/*
* Clean up server info structure resources.
*/
@@ -3451,25 +3471,41 @@
int LockRegion(int fd, int cmd, int type, off_t offset, int whence, off_t len)
{
struct flock lock;
+ int res;
lock.l_type = type; /* F_RDLCK, F_WRLCK, F_UNLCK */
lock.l_start = offset; /* byte offset, relative to whence */
lock.l_whence = whence; /* SEEK_SET, SEET_CUR, SEEK_END */
lock.l_len = len; /* # of bytes, (0 indicates to EOF) */
- - return (fcntl(fd, cmd, &lock));
+ /* Don't be fooled into thinking we've set a lock when we've
+ merely caught a signal. */
+ while ((res = fcntl(fd, cmd, &lock)) == -1 && errno == EINTR)
+ ;
+ return res;
}
+/* Ways to lock an entire file... */
+
+/* Shared locks: allows other shared locks, but no exclusive locks. */
+/* Set a shared lock, no wait, failure->errno==EACCES. */
#define ReadLock(fd) LockRegion(fd, F_SETLK, F_RDLCK, 0, SEEK_SET, 0)
+/* Set a shared lock, wait until you have it. */
#define ReadwLock(fd) LockRegion(fd, F_SETLKW, F_RDLCK, 0, SEEK_SET, 0)
+
+/* Exclusive locks: allows no other locks. */
+/* Set an exclusive lock, no wait, failure->errno==EACCES. */
#define WriteLock(fd) LockRegion(fd, F_SETLK, F_WRLCK, 0, SEEK_SET, 0)
+/* Set a shared lock, wait until you have it. */
#define WritewLock(fd) LockRegion(fd, F_SETLKW, F_WRLCK, 0, SEEK_SET, 0)
+
+/* Remove a shared or exclusive lock, no wait, failure->errno=EACCES. */
#define Unlock(fd) LockRegion(fd, F_SETLK, F_UNLCK, 0, SEEK_SET, 0)
/****************************************************************************/
/*
* Opcodes for Server/ProcMgr communication
*/
- -#define PLEASE_START 49 /* start application */
+#define PLEASE_START 49 /* start dynamic application */
#define CONN_TIMEOUT 50 /* start another copy of application */
#define REQ_COMPLETE 51 /* do some data analysis */
/*
@@ -3560,7 +3596,7 @@
struct stat statbuf;
int recs = 0, fd, i;
char *buf=NULL, opcode;
- - char *buf2=NULL;
+ char *lockFileName=NULL;
char *ptr1=NULL, *ptr2=NULL;
char execName[TMP_BUFSIZ];
unsigned long qsec = 0, ctime = 0, now = time(NULL);
@@ -3680,12 +3716,12 @@
}
OS_FreeIpcAddr(ipcAddrPtr);
/* don't forget to create a lock file for this app */
- - buf2 = MakeLockFileName(execName);
- - fd = open(buf2, O_WRONLY | O_CREAT | O_TRUNC,
+ lockFileName = MakeLockFileName(execName);
+ fd = open(lockFileName, O_WRONLY | O_CREAT | O_TRUNC,
S_IRUSR | S_IWUSR);
- - free(buf2);
ASSERT(fd>0);
close(fd);
+ free(lockFileName);
} else {
if(opcode==PLEASE_START) {
/* repeated calls to PLEASE_START, ignore */
@@ -3859,7 +3895,7 @@
if((lockFd = open(lockFileName, O_RDWR))<0) {
/*
* If we need to kill an application and the
- - * corresponding lock file does not exists, then
+ * corresponding lock file does not exist, then
* that means we are in big trouble here
*/
continue;
@@ -3883,18 +3919,21 @@
* directly.
*/
if((pid=fork())<0) {
- - Unlock(lockFd);
+ Free(funcData->lockFileName);
+ Free(funcData);
return;
} else if(pid==0) {
/* child */
BlockingKill(funcData);
} else {
/* parent */
+ Free(funcData->lockFileName);
+ Free(funcData);
}
} else {
kill(s->procInfo[i].pid, SIGTERM);
Unlock(lockFd);
- - break;
+ break;
}
}
}
@@ -3935,7 +3974,7 @@
*/
static int caughtSigTerm = FALSE;
static int caughtSigChld = FALSE;
- -static int caughtSigUsr1 = FALSE;
+static int caughtSigUsr2 = FALSE;
static char *errorLogPathname = NULL;
static sigset_t signalsToBlock;
@@ -3966,7 +4005,7 @@
} else if(signo == SIGCHLD) {
caughtSigChld = TRUE;
} else if(signo == SIGUSR2 || signo == SIGALRM) {
- - caughtSigUsr1 = TRUE;
+ caughtSigUsr2 = TRUE;
}
}
@@ -4146,7 +4185,7 @@
if(caughtSigTerm) {
goto ProcessSigTerm;
}
- - if((!caughtSigChld) && (!caughtSigUsr1)) {
+ if((!caughtSigChld) && (!caughtSigUsr2)) {
/*
* Enable signals and wait. The call to sigsuspend
* breaks the critical region into two, so caughtSigChld
@@ -4158,8 +4197,8 @@
}
callWaitPid = caughtSigChld;
caughtSigChld = FALSE;
- - callDynamicProcs = caughtSigUsr1;
- - caughtSigUsr1 = FALSE;
+ callDynamicProcs = caughtSigUsr2;
+ caughtSigUsr2 = FALSE;
sigprocmask(SIG_UNBLOCK, &signalsToBlock, NULL);
/*
@@ -4183,9 +4222,6 @@
}
if(!callWaitPid) {
- - /*
- - * Must be time to restart somebody.
- - */
continue;
}
@@ -4342,8 +4378,8 @@
* DoesAnyFileExist --
*
* Check if the file is present in the file system in the
- - * directory specified by $ircDynamicDir. Also check that
- - * a given filename of the certain type, such as regular or
+ * directory specified by ipcDynamicDir. Also check that
+ * a given filename is of the given type, such as regular or
* socket (FIFO).
*
* Results:
@@ -4413,11 +4449,11 @@
* An Apache module initializer, called by the Apache core
* after reading the server config.
*
- - * Start the process manager no matter what, since there maybe a
+ * Start the process manager no matter what, since there may be a
* request for dynamic FastCGI applications without any being
* configured as static applications. Also, check for the existence
* and create if necessary a subdirectory into which all dynamic
- - * sockets will go into.
+ * sockets will go.
*
*----------------------------------------------------------------------
*/
@@ -5589,7 +5625,7 @@
if (serverInfoPtr == NULL) {
/*
* Either this application does not exist, or it is
- - * the invokation of the dynamic fastcgi
+ * the invocation of the dynamic fastcgi
* application.
*/
uid = (user_id == (uid_t) -1) ? geteuid() : user_id;
------- End of Forwarded Message