jech / polipo

The Polipo caching HTTP proxy
http://www.pps.jussieu.fr/~jch/software/polipo/
MIT License
1.81k stars 354 forks source link

correct pid file handling/singleton daemon #20

Closed dmig closed 10 years ago

dmig commented 10 years ago

Current pid file handling is not correct: it prevents daemon to start, if pid file exists, not checking whether daemon is actually running.

Here is a patch correcting this issue:

diff --git a/util.c b/util.c
index 8fc30e5..ddeb770 100644
--- a/util.c
+++ b/util.c
@@ -591,12 +591,26 @@ writePid(char *pidfile)
     int fd, n, rc;
     char buf[16];

-    fd = open(pidfile, O_WRONLY | O_CREAT | O_EXCL, 0666);
+    fd = open(pidfile, O_WRONLY | O_CREAT, 0644);
     if(fd < 0) {
                do_log_error(L_ERROR, errno,
                                        "Couldn't create pid file %s", pidfile);
                exit(1);
     }
+       rc = lockf(fd, F_TLOCK, 0);
+       if(rc) {
+               n = errno;
+               if(EACCES == n || EAGAIN == n) {
+                       do_log_error(L_ERROR, 0,
+                                               "Another instance is alredy running");
+                       exit(1);
+               } else {
+                       do_log_error(L_ERROR, n,
+                                               "Couldn't create pid file %s", pidfile);
+                       exit(1);
+               }
+       }
+
     n = snprintf(buf, 16, "%ld\n", (long)getpid());
     if(n < 0 || n >= 16) {
         close(fd);
jech commented 10 years ago

Thanks for the patch, but I'm not applying that. First, lockf is a broken design, it interacts badly with fork (and Polipo uses fork, see local.c, forbidden.c) -- flock would be a better choice.

Second, I think it's overkill. If there's a stray lock file, something went wrong, so I'd expect the system administrator or the init script to do some checking.

Sorry for that.