daemons: lib: changed lock from flock() to fcntl()

`flock()` was a good solution to lock the pid file in the main program (before forking) and keep the lock after the fork (pass it to child process). But unfortunately, `flock()` is not supported on `Solaris` systems, and therefore cannot be used here. `fcntl()` can do the same base job and is supported on all the platforms Bareos supports. But the issue with it, is that the lock is not kept and passed to the child process when forking. That is why the locking code was moved inside the forked daemon when writing to the pid file.
bareos · Oct 21, 2021 · 50451d1 · 50451d1
1 parent bc2655b
commit 50451d1
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 42 deletions.
diff --git a/core/src/dird/dird.cc b/core/src/dird/dird.cc
@@ -339,7 +339,7 @@ int main(int argc, char* argv[])
 
   int pidfile_fd = -1;
   if (!test_config && background && pidfile_path) {
-    pidfile_fd = CreatePidFile("bareos-dir", pidfile_path);
+    pidfile_fd = CreatePidFile(pidfile_path);
   }
 
   // See if we want to drop privs.
@@ -379,7 +379,7 @@ int main(int argc, char* argv[])
 
   if (!test_config) { /* we don't need to do this block in test mode */
     if (background) {
-      daemon_start(pidfile_fd, pidfile_path);
+      daemon_start("bareos-dir", pidfile_fd, pidfile_path);
       InitStackDump(); /* grab new pid */
     }
   }

diff --git a/core/src/filed/filed.cc b/core/src/filed/filed.cc
@@ -223,7 +223,7 @@ int main(int argc, char* argv[])
 
   int pidfile_fd = 0;
   if (!foreground && !test_config && pidfile_path) {
-    pidfile_fd = CreatePidFile("bareos-fd", pidfile_path);
+    pidfile_fd = CreatePidFile(pidfile_path);
   }
 
   // See if we want to drop privs.
@@ -262,7 +262,7 @@ int main(int argc, char* argv[])
   }
 
   if (!foreground && !test_config) {
-    daemon_start(pidfile_fd, pidfile_path);
+    daemon_start("bareos-fd", pidfile_fd, pidfile_path);
     InitStackDump(); /* set new pid */
   }
 

diff --git a/core/src/lib/bsys.cc b/core/src/lib/bsys.cc
@@ -417,6 +417,35 @@ int b_strerror(int errnum, char* buf, size_t bufsiz)
   return status;
 }
 
+
+static void LockPidFile(const char* progname,
+                        int pidfile_fd,
+                        const char* pidfile_path)
+{
+#if !defined(HAVE_WIN32)
+  struct flock fl;
+
+  fl.l_type = F_WRLCK;
+  fl.l_whence = SEEK_SET;
+  fl.l_start = 0;
+  fl.l_len = 0;
+
+  if (fcntl(pidfile_fd, F_SETLK, &fl)) {
+    if (errno == EAGAIN || errno == EACCES) {
+      BErrNo be;
+      Emsg2(M_ERROR_TERM, 0,
+            _("PID file '%s' is locked; probably '%s' is already running. "
+              "ERR=%s\n"),
+            pidfile_path, progname, be.bstrerror());
+
+    } else {
+      BErrNo be;
+      Emsg2(M_ERROR_TERM, 0, _("Unable to lock PID file '%s'. ERR=%s\n"),
+            pidfile_path, be.bstrerror());
+    }
+  }
+#endif
+}
 /*
    The content of this function (CreatePidFile) was inspired and modified to fit
    current needs from filelock/create_pid_file.c (Listing 55-4, page 1143), an
@@ -429,15 +458,15 @@ int b_strerror(int errnum, char* buf, size_t bufsiz)
 
 #define CPF_CLOEXEC 1
 
-int CreatePidFile(const char* progname, const char* pidFile)
+int CreatePidFile(const char* pidfile_path)
 {
 #if !defined(HAVE_WIN32)
   int pidfd;
 
-  pidfd = open(pidFile, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
+  pidfd = open(pidfile_path, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
   if (pidfd == -1) {
     BErrNo be;
-    Emsg2(M_ERROR_TERM, 0, _("Cannot open pid file. %s ERR=%s\n"), pidFile,
+    Emsg2(M_ERROR_TERM, 0, _("Cannot open pid file. %s ERR=%s\n"), pidfile_path,
           be.bstrerror());
   }
 
@@ -454,30 +483,15 @@ int CreatePidFile(const char* progname, const char* pidFile)
     if (flags == -1) {
       BErrNo be;
       Emsg2(M_ERROR_TERM, 0, _("Could not get flags for PID file %s. ERR=%s\n"),
-            pidFile, be.bstrerror());
+            pidfile_path, be.bstrerror());
     }
 
     flags |= FD_CLOEXEC; /* Turn on FD_CLOEXEC */
 
     if (fcntl(pidfd, F_SETFD, flags) == -1) /* Update flags */ {
       BErrNo be;
       Emsg2(M_ERROR_TERM, 0, _("Could not get flags for PID file %s. ERR=%s\n"),
-            pidFile, be.bstrerror());
-    }
-  }
-
-  if (flock(pidfd, LOCK_EX | LOCK_NB) == -1) {
-    if (errno == EAGAIN || errno == EACCES) {
-      BErrNo be;
-      Emsg2(M_ERROR_TERM, 0,
-            _("PID file '%s' is locked; probably '%s' is already running. "
-              "ERR=%s\n"),
-            pidFile, progname, be.bstrerror());
-
-    } else {
-      BErrNo be;
-      Emsg2(M_ERROR_TERM, 0, _("Unable to lock PID file '%s'. ERR=%s\n"),
-            pidFile, be.bstrerror());
+            pidfile_path, be.bstrerror());
     }
   }
   return pidfd;
@@ -494,27 +508,34 @@ int CreatePidFile(const char* progname, const char* pidFile)
    License, version 3.
 */
 void WritePidFile(const int& pidfile_fd,
-                  const char* pidFile,
-                  const pid_t& fork_pid)
+                  const char* pidfile_path,
+                  const pid_t& fork_pid,
+                  const char* progname)
 {
+#if !defined(HAVE_WIN32)
   int buf_size = 100;
   char buf[buf_size];
+
+  LockPidFile(progname, pidfile_fd, pidfile_path);
+
   if (ftruncate(pidfile_fd, 0) == -1) {
     BErrNo be;
     Emsg2(M_ERROR_TERM, 0, _("Could not truncate PID file '%s'. ERR=%s\n"),
-          pidFile, be.bstrerror());
+          pidfile_path, be.bstrerror());
   }
 
   snprintf(buf, buf_size, "%d\n", fork_pid);
-  if (write(pidfile_fd, buf, strlen(buf)) != strlen(buf)) {
+  if (write(pidfile_fd, buf, strlen(buf))
+      != static_cast<ssize_t>(strlen(buf))) {
     BErrNo be;
-    Emsg2(M_ERROR_TERM, 0, _("Writing to PID file '%s'. ERR=%s\n"), pidFile,
-          be.bstrerror());
+    Emsg2(M_ERROR_TERM, 0, _("Writing to PID file '%s'. ERR=%s\n"),
+          pidfile_path, be.bstrerror());
   }
+#endif
 }
 
 // Delete the pid file if we created it
-int DeletePidFile(char* pidfile_path)
+int DeletePidFile(const char* pidfile_path)
 {
 #if !defined(HAVE_WIN32)
   unlink(pidfile_path);

diff --git a/core/src/lib/bsys.h b/core/src/lib/bsys.h
@@ -36,11 +36,12 @@ int cstrlen(const char* str);
 int Bsnprintf(char* str, int32_t size, const char* format, ...);
 int Bvsnprintf(char* str, int32_t size, const char* format, va_list ap);
 int PoolSprintf(char* pool_buf, const char* fmt, ...);
-int CreatePidFile(const char* progname, const char* pidFile);
+int CreatePidFile(const char* pidfile_path);
 void WritePidFile(const int& pidfile_fd,
-                  const char* pidFile,
-                  const pid_t& forkpid);
-int DeletePidFile(char* pidfile_path);
+                  const char* pidfile_path,
+                  const pid_t& forkpid,
+                  const char* progname);
+int DeletePidFile(const char* pidfile_path);
 void drop(char* uid, char* gid, bool keep_readall_caps);
 int Bmicrosleep(int32_t sec, int32_t usec);
 char* bfgets(char* s, int size, FILE* fd);

diff --git a/core/src/lib/daemon.cc b/core/src/lib/daemon.cc
@@ -38,7 +38,10 @@
 
 #if defined(HAVE_WIN32)
 
-void daemon_start(int pidfile_fd, char* pidfile_path) { return }
+void daemon_start(const char* progname, int pidfile_fd, char* pidfile_path)
+{
+  return;
+}
 
 #else  // !HAVE_WIN32
 
@@ -61,7 +64,7 @@ static void SetupStdFileDescriptors()
 }
 #  endif  // DEVELOPER
 
-void daemon_start(int pidfile_fd, char* pidfile_path)
+void daemon_start(const char* progname, int pidfile_fd, char* pidfile_path)
 {
   Dmsg0(900, "Enter daemon_start\n");
 
@@ -71,7 +74,12 @@ void daemon_start(int pidfile_fd, char* pidfile_path)
     case 0:
       setsid();
       umask(umask(0) | S_IWGRP | S_IROTH | S_IWOTH);
+
+      if (pidfile_path) {
+        WritePidFile(pidfile_fd, pidfile_path, getpid(), progname);
+      }
       SetupStdFileDescriptors();
+
       break;
     case -1: {
       BErrNo be;
@@ -80,7 +88,6 @@ void daemon_start(int pidfile_fd, char* pidfile_path)
       break;
     }
     default:
-      if (pidfile_path) { WritePidFile(pidfile_fd, pidfile_path, forkpid); }
       exit(0);
   }
 

diff --git a/core/src/lib/daemon.h b/core/src/lib/daemon.h
@@ -21,6 +21,6 @@
 #ifndef BAREOS_LIB_DAEMON_H_
 #define BAREOS_LIB_DAEMON_H_
 
-void daemon_start(int pidfile_fd, char* pidfilepath);
+void daemon_start(const char* progname, int pidfile_fd, char* pidfilepath);
 
 #endif  // BAREOS_LIB_DAEMON_H_
diff --git a/core/src/stored/stored.cc b/core/src/stored/stored.cc
@@ -254,7 +254,7 @@ int main(int argc, char* argv[])
 
   int pidfile_fd = 0;
   if (!foreground && !test_config && pidfile_path) {
-    pidfile_fd = CreatePidFile("bareos-sd", pidfile_path);
+    pidfile_fd = CreatePidFile(pidfile_path);
   }
 
   // See if we want to drop privs.
@@ -292,8 +292,8 @@ int main(int argc, char* argv[])
   }
 
   if (!foreground && !test_config) {
-    daemon_start(pidfile_fd, pidfile_path); /* become daemon */
-    InitStackDump();                        /* pick up new pid */
+    daemon_start("bareos-sd", pidfile_fd, pidfile_path); /* become daemon */
+    InitStackDump();                                     /* pick up new pid */
   }
 
   if (InitCrypto() != 0) {