Merge branch 'cleanup_pidfile' of https://github.com/mattikus/bcfg2 into maint

2 files changed, 41 insertions, 23 deletions

diff --git a/src/lib/Bcfg2/Reporting/Collector.py b/src/lib/Bcfg2/Reporting/Collector.py
index 8ca145f16..8e2fe1cb1 100644
--- a/src/lib/Bcfg2/Reporting/Collector.py
+++ b/src/lib/Bcfg2/Reporting/Collector.py
@@ -1,3 +1,4 @@
+import os
 import sys
 import atexit
 import daemon
@@ -6,13 +7,12 @@ import time
 import traceback
 import threading
 
-# pylint: disable=E0611
 from lockfile import LockFailed, LockTimeout
+# pylint: disable=E0611
 try:
-    from lockfile.pidlockfile import PIDLockFile
-    from lockfile import Error as PIDFileError
+    from daemon.pidfile import TimeoutPIDLockFile
 except ImportError:
-    from daemon.pidlockfile import PIDLockFile, PIDFileError
+    from daemon.pidlockfile import TimeoutPIDLockFile
 # pylint: enable=E0611
 
 import Bcfg2.Logger
@@ -118,25 +118,31 @@ class ReportingCollector(object):
 
         if self.setup['daemon']:
             self.logger.debug("Daemonizing")
+            self.context.pidfile = TimeoutPIDLockFile(self.setup['daemon'],
+                                                      acquire_timeout=5)
+            # Attempt to ensure lockfile is able to be created and not stale
             try:
-                self.context.pidfile = PIDLockFile(self.setup['daemon'])
-                self.context.open()
+                self.context.pidfile.acquire()
             except LockFailed:
                 self.logger.error("Failed to daemonize: %s" %
                                   sys.exc_info()[1])
                 self.shutdown()
                 return
             except LockTimeout:
-                self.logger.error("Failed to daemonize: "
-                                  "Failed to acquire lock on %s" %
-                                  self.setup['daemon'])
-                self.shutdown()
-                return
-            except PIDFileError:
-                self.logger.error("Error writing pid file: %s" %
-                    traceback.format_exc().splitlines()[-1])
-                self.shutdown()
-                return
+                try: # attempt to break the lock
+                    os.kill(self.context.pidfile.read_pid(), 0)
+                except (OSError, TypeError): # No process with locked PID
+                    self.context.pidfile.break_lock()
+                else:
+                    self.logger.error("Failed to daemonize: "
+                                      "Failed to acquire lock on %s" %
+                                      self.setup['daemon'])
+                    self.shutdown()
+                    return
+            else:
+                self.context.pidfile.release()
+
+            self.context.open()
             self.logger.info("Starting daemon")
 
         self.transport.start_monitor(self)
diff --git a/src/lib/Bcfg2/Server/BuiltinCore.py b/src/lib/Bcfg2/Server/BuiltinCore.py
index 93da767c7..29beb35d5 100644
--- a/src/lib/Bcfg2/Server/BuiltinCore.py
+++ b/src/lib/Bcfg2/Server/BuiltinCore.py
@@ -1,5 +1,6 @@
 """ The core of the builtin Bcfg2 server. """
 
+import os
 import sys
 import time
 import socket
@@ -83,19 +84,30 @@ class Core(BaseCore):
     def _daemonize(self):
         """ Open :attr:`context` to drop privileges, write the PID
         file, and daemonize the server core. """
+        # Attempt to ensure lockfile is able to be created and not stale
         try:
-            self.context.open()
-            self.logger.info("%s daemonized" % self.name)
-            return True
+            self.context.pidfile.acquire()
         except LockFailed:
             err = sys.exc_info()[1]
             self.logger.error("Failed to daemonize %s: %s" % (self.name, err))
             return False
         except LockTimeout:
-            err = sys.exc_info()[1]
-            self.logger.error("Failed to daemonize %s: Failed to acquire lock "
-                              "on %s" % (self.name, self.setup['daemon']))
-            return False
+            try:  # attempt to break the lock
+                os.kill(self.context.pidfile.read_pid(), 0)
+            except (OSError, TypeError):  # No process with locked PID
+                self.context.pidfile.break_lock()
+            else:
+                err = sys.exc_info()[1]
+                self.logger.error("Failed to daemonize %s: Failed to acquire"
+                                  "lock on %s" % (self.name,
+                                                  self.setup['daemon']))
+                return False
+        else:
+            self.context.pidfile.release()
+
+        self.context.open()
+        self.logger.info("%s daemonized" % self.name)
+        return True
 
     def _run(self):
         """ Create :attr:`server` to start the server listening. """