On Tue, 5 Apr 2005, Andrew Bartlett wrote:
What Samba does on it's pid file is to check there is a fcntl() lock on
the file. That way, we know that the process with that PID is also
Samba, not a re-used PID.
Good idea. Except for the minor detail that it wastes one filedescriptor
to keep the lock file open.. Proposed patch attached implementing a fcntl
lock guarantee, but I am not entirely convinced this is worth wasting the
filedescriptor on.
Note: Squid does already verify that the pid really exists like Nickolay
proposed, not only that the pid file exists. See checkRunningPid(). The
attached patch is only to extend this with a guarnatee like described
above. There is however some broken third-party init scripts floating
around mismanaging both the pid files and their own service lock files.
The major Linux vendor is quite notable in shipping such broken init
scripts..
Regards
Henrik? ChangeLog.cvs
Index: src/main.c
===
RCS file: /server/cvs-server/squid/squid/src/main.c,v
retrieving revision 1.345.2.22
diff -u -p -r1.345.2.22 main.c
--- src/main.c 19 Mar 2005 23:56:23 - 1.345.2.22
+++ src/main.c 5 Apr 2005 22:12:50 -
@@ -1021,11 +1021,7 @@ SquidShutdown(void *unused)
#if MEM_GEN_TRACE
log_trace_done();
#endif
-if (Config.pidFilename strcmp(Config.pidFilename, none) != 0) {
- enter_suid();
- safeunlink(Config.pidFilename, 0);
- leave_suid();
-}
+removePidFile();
debug(1, 1) (Squid Cache (Version %s): Exiting normally.\n,
version_string);
if (debug_log)
Index: src/protos.h
===
RCS file: /server/cvs-server/squid/squid/src/protos.h,v
retrieving revision 1.420.2.32
diff -u -p -r1.420.2.32 protos.h
--- src/protos.h3 Apr 2005 23:08:48 - 1.420.2.32
+++ src/protos.h5 Apr 2005 22:12:50 -
@@ -1067,13 +1067,14 @@ extern void leave_suid(void);
extern void enter_suid(void);
extern void no_suid(void);
extern void writePidFile(void);
+extern pid_t readPidFile(void);
+extern void removePidFile(void);
extern void setSocketShutdownLifetimes(int);
extern void setMaxFD(void);
extern time_t getCurrentTime(void);
extern int percent(int, int);
extern double dpercent(double, double);
extern void squid_signal(int sig, SIGHDLR *, int flags);
-extern pid_t readPidFile(void);
extern struct in_addr inaddrFromHostent(const struct hostent *hp);
extern int intAverage(int, int, int, int);
extern double doubleAverage(double, double, int, int);
Index: src/tools.c
===
RCS file: /server/cvs-server/squid/squid/src/tools.c,v
retrieving revision 1.213.2.14
diff -u -p -r1.213.2.14 tools.c
--- src/tools.c 26 Mar 2005 23:27:10 - 1.213.2.14
+++ src/tools.c 5 Apr 2005 22:12:50 -
@@ -585,6 +585,7 @@ no_suid(void)
#endif
}
+static int pid_file_fd = -1;
void
writePidFile(void)
{
@@ -596,6 +597,9 @@ writePidFile(void)
return;
if (!strcmp(Config.pidFilename, none))
return;
+if (pid_file_fd 0)
+ file_close(pid_file_fd);
+pid_file_fd = -1;
enter_suid();
old_umask = umask(022);
fd = file_open(f, O_WRONLY | O_CREAT | O_TRUNC | O_TEXT);
@@ -606,9 +610,22 @@ writePidFile(void)
debug_trap(Could not write pid file);
return;
}
+#ifdef F_SETLK
+{
+ struct flock lk;
+ memset(lk, 0, sizeof(lk));
+ lk.l_type = F_WRLCK;
+ lk.l_whence = SEEK_SET;
+ if (fcntl(fd, F_SETLK, lk) != 0) {
+ debug_trap(PID file in use\n);
+ file_close(fd);
+ return;
+ }
+}
+#endif
snprintf(buf, 32, %d\n, (int) getpid());
FD_WRITE_METHOD(fd, buf, strlen(buf));
-file_close(fd);
+pid_file_fd = fd;
}
@@ -625,21 +642,50 @@ readPidFile(void)
exit(1);
}
pid_fp = fopen(f, r);
-if (pid_fp != NULL) {
- pid = 0;
- if (fscanf(pid_fp, %d, i) == 1)
- pid = (pid_t) i;
- fclose(pid_fp);
-} else {
+if (pid_fp == NULL) {
if (errno != ENOENT) {
fprintf(stderr, %s: ERROR: Could not read pid file\n, appname);
fprintf(stderr, \t%s: %s\n, f, xstrerror());
exit(1);
}
+ return 0;
}
+#ifdef F_GETLK
+{
+ struct flock lk;
+ memset(lk, 0, sizeof(lk));
+ lk.l_type = F_WRLCK;
+ lk.l_whence = SEEK_SET;
+ fcntl(fileno(pid_fp), F_GETLK, lk);
+ if (lk.l_type == F_UNLCK) {
+ fclose(pid_fp);
+ return 0;
+ }
+}
+#endif
+pid = 0;
+if (fscanf(pid_fp, %d, i) == 1)
+ pid = (pid_t) i;
+fclose(pid_fp);
return pid;
}
+void
+removePidFile(void)
+{
+const char *f = NULL;
+if ((f = Config.pidFilename) == NULL)
+ return;
+if (!strcmp(Config.pidFilename, none))
+ return;
+if (pid_file_fd 0) {
+
