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 e7ee56c..e12bc14 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 8cbcfaf..14436f3 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 ce4a53b..4c947a4 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);