worker: Prevent worker from being responsible for pidfile deletion.

Currently we are creating the worker process after creation of the pidfile.
This means that the responsibility of deleting the pidfile after process
termination rests with the worker process.

When we restart openvswitch using the startup scripts, we SIGTERM the main
process and once it is cleaned up, we start ovs-vswitchd again. This results
in a race condition. The new ovs-vswitchd will create a pidfile because it is
unlocked. But, if the old worker process exits after the start of new
ovs-vswitchd, it will simply delete the pidfile underneath the new ovs-vswitchd.
This will eventually result in multiple ovs-vswitchd daemons.

This patch gives the responsibility of deleting the pidfile to the main
process.

Bug #16669.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>

diff --git a/lib/daemon.c b/lib/daemon.c
index e7ee56ce1207aeda07b0688c81a7fb6e93080440..e12bc14ae351944f564b7e80fd7be73761032885 100644 (file)
--- a/lib/daemon.c
+++ b/lib/daemon.c
@@ -163,6 +163,26 @@ daemon_save_fd(int fd)
     save_fds[fd] = true;
 }
 
+/* Unregisters pidfile from being unlinked when the program terminates via
+* exit() or a fatal signal. */
+void
+remove_pidfile_from_unlink(void)
+{
+    if (pidfile) {
+        fatal_signal_remove_file_to_unlink(pidfile);
+    }
+}
+
+/* Registers pidfile to be unlinked when the program terminates via exit() or a
+ * fatal signal. */
+void
+add_pidfile_to_unlink(void)
+{
+    if (pidfile) {
+        fatal_signal_add_file_to_unlink(pidfile);
+    }
+}
+
 /* If a pidfile has been configured, creates it and stores the running
  * process's pid in it.  Ensures that the pidfile will be deleted when the
  * process exits. */
@@ -240,8 +260,6 @@ make_pidfile(void)
     pidfile_dev = s.st_dev;
     pidfile_ino = s.st_ino;
     free(tmpfile);
-    free(pidfile);
-    pidfile = NULL;
 }
 
 /* If configured with set_pidfile() or set_detach(), creates the pid file and
@@ -529,6 +547,11 @@ daemonize_start(void)
 void
 daemonize_complete(void)
 {
+    if (pidfile) {
+        free(pidfile);
+        pidfile = NULL;
+    }
+
     if (!detached) {
         detached = true;
 

diff --git a/lib/daemon.h b/lib/daemon.h
index 8cbcfafe2442472eb1e16ea65269a2980ea498af..14436f311eeb9da7783da837517e235d9224df83 100644 (file)
--- a/lib/daemon.h
+++ b/lib/daemon.h
@@ -65,6 +65,8 @@ void set_detach(void);
 bool get_detach(void);
 void daemon_set_monitor(void);
 void daemon_save_fd(int fd);
+void remove_pidfile_from_unlink(void);
+void add_pidfile_to_unlink(void);
 void daemonize(void);
 void daemonize_start(void);
 void daemonize_complete(void);

diff --git a/lib/worker.c b/lib/worker.c
index ce4a53b26051505d5e995b335c7cbee87e72adef..4c947a422aa057751139c1e6130e0005145ca128 100644 (file)
--- a/lib/worker.c
+++ b/lib/worker.c
@@ -101,6 +101,9 @@ worker_start(void)
     xset_nonblocking(work_fds[0]);
     xset_nonblocking(work_fds[1]);
 
+    /* Don't let the worker process own the responsibility to delete
+     * the pidfile.  Register it again after the fork. */
+    remove_pidfile_from_unlink();
     if (!fork_and_clean_up()) {
         /* In child (worker) process. */
         daemonize_post_detach();
@@ -110,6 +113,7 @@ worker_start(void)
     }
 
     /* In parent (main) process. */
+    add_pidfile_to_unlink();
     close(work_fds[1]);
     client_sock = work_fds[0];
     rxbuf_init(&client_rx);