Revert "lockfile improvements"

gunicorn/arbiter.py

@@ -138,7 +138,16 @@ class Arbiter(object):
             self.LISTENERS, need_lock = create_sockets(self.cfg, self.log)
 
         if need_lock:
-            self.acquire_lockfile()
+            if not self.LOCK_FILE:
+                # reuse the lockfile if already set
+                if 'GUNICORN_LOCK' in os.environ:
+                    lock_path = os.environ.get('GUNICORN_LOCK')
+                else:
+                    lock_path = self.cfg.lockfile
+
+                self.LOCK_FILE = LockFile(lock_path)
+            # add us to the shared lock
+            self.LOCK_FILE.lock()
 
         listeners_str = ",".join([str(l) for l in self.LISTENERS])
         self.log.debug("Arbiter booted")
@@ -341,13 +350,17 @@ class Arbiter(object):
         :attr graceful: boolean, If True (the default) workers will be
         killed gracefully  (ie. trying to wait for the current connection)
         """
-        released = self.try_release_lockfile()
+        locked = False
+        if self.LOCK_FILE:
+            self.LOCK_FILE.unlock()
+            locked = self.LOCK_FILE.locked()
+
+            # delete the lock file if needed
+            if not locked and 'GUNICORN_LOCK' in os.environ:
+                del os.environ['GUNICORN_LOCK']
+
         for l in self.LISTENERS:
-            l.close()
-
-            if released:
-                l.destroy()
-
+            l.close(locked)
         self.LISTENERS = []
         sig = signal.SIGTERM
         if not graceful:
@@ -440,36 +453,6 @@ class Arbiter(object):
         # manage workers
         self.manage_workers()
 
-    def acquire_lockfile(self):
-        """\
-        Acquire the lock file
-        """
-        if not self.LOCK_FILE:
-            # reuse the lockfile if already set
-            if 'GUNICORN_LOCK' in os.environ:
-                lock_path = os.environ.get('GUNICORN_LOCK')
-            else:
-                lock_path = self.cfg.lockfile
-
-            self.LOCK_FILE = LockFile(lock_path)
-        # add us to the shared lock
-        self.LOCK_FILE.acquire()
-
-    def try_release_lockfile(self):
-        """\
-        Try to release the lock file
-        """
-        released = False
-        if self.LOCK_FILE:
-            released = self.LOCK_FILE.release()
-
-            # delete the lock file if needed
-            if released and 'GUNICORN_LOCK' in os.environ:
-                del os.environ['GUNICORN_LOCK']
-
-        return released
-
-
     def murder_workers(self):
         """\
         Kill unused/idle workers

gunicorn/lockfile.py

@@ -49,27 +49,24 @@ class LockFile(object):
         self._lockfile = open(self.fname, 'w+b')
         # set permissions to -rw-r--r--
         os.chmod(self.fname, 420)
-        self._released = True
+        self._locked = False
 
-    def acquire(self):
+    def lock(self):
         _lock(self._lockfile.fileno())
-        self._released = False
+        self._locked = True
 
-    def release(self):
-        if self.released():
-            return True
+    def unlock(self):
+        if not self.locked():
+            return
 
         if _unlock(self._lockfile.fileno()):
             self._lockfile.close()
             util.unlink(self.fname)
             self._lockfile = None
-            self._released = True
-            return True
+            self._locked = False
 
-        return False
-
-    def released(self):
-        return self._lockfile is None or self._released
+    def locked(self):
+        return self._lockfile is not None and self._locked
 
     def name(self):
         return self.fname

gunicorn/sock.py

@@ -53,7 +53,7 @@ class BaseSocket(object):
     def bind(self, sock):
         sock.bind(self.cfg_addr)
 
-    def close(self):
+    def close(self, locked=False):
         if self.sock is None:
             return
 
@@ -64,9 +64,6 @@ class BaseSocket(object):
 
         self.sock = None
 
-    def destroy(self):
-        pass
-
 
 class TCPSocket(BaseSocket):
 
@@ -123,12 +120,10 @@ class UnixSocket(BaseSocket):
         util.chown(self.cfg_addr, self.conf.uid, self.conf.gid)
         os.umask(old_umask)
 
-    def close(self):
-        super(UnixSocket, self).close()
-
-    def destroy(self):
-        if self.parent == os.getpid():
+    def close(self, locked=False):
+        if self.parent == os.getpid() and not locked:
             os.unlink(self.cfg_addr)
+        super(UnixSocket, self).close()
 
 
 def _sock_type(addr):

tests/test_arbiter.py

@@ -35,8 +35,8 @@ def test_arbiter_shutdown_closes_listeners():
     listener2 = mock.Mock()
     arbiter.LISTENERS = [listener1, listener2]
     arbiter.stop()
-    listener1.close.assert_called_with()
-    listener2.close.assert_called_with()
+    listener1.close.assert_called_with(False)
+    listener2.close.assert_called_with(False)
 
 
 class PreloadedAppWithEnvSettings(DummyApplication):

tests/test_lockfile.py

@@ -6,10 +6,10 @@ from gunicorn.util import tmpfile
 def test_lockfile():
     lockname = tmpfile(prefix="gunicorn-tests", suffix=".lock")
     lock_file = LockFile(lockname)
-    assert lock_file.released() == True
+    assert lock_file.locked() == False
     assert os.path.exists(lockname)
-    lock_file.acquire()
-    assert lock_file.released() == False
-    lock_file.release()
-    assert lock_file.released() == True
+    lock_file.lock()
+    assert lock_file.locked() == True
+    lock_file.unlock()
+    assert lock_file.locked() == False
     assert os.path.exists(lockname) == False

tests/test_sock.py

@@ -11,10 +11,19 @@ from gunicorn import sock
 @mock.patch('socket.fromfd')
 def test_unix_socket_close_delete_if_exlock(fromfd, unlink, getpid):
     gsock = sock.UnixSocket('test.sock', mock.Mock(), mock.Mock(), mock.Mock())
-    gsock.destroy()
+    gsock.close(False)
     unlink.assert_called_with('test.sock')
 
 
+@mock.patch('os.getpid')
+@mock.patch('os.unlink')
+@mock.patch('socket.fromfd')
+def test_unix_socket_close_keep_if_no_exlock(fromfd, unlink, getpid):
+    gsock = sock.UnixSocket('test.sock', mock.Mock(), mock.Mock(), mock.Mock())
+    gsock.close(True)
+    unlink.assert_not_called()
+
+
 @mock.patch('os.getpid')
 @mock.patch('os.unlink')
 @mock.patch('socket.fromfd')
@@ -23,5 +32,5 @@ def test_unix_socket_not_deleted_by_worker(fromfd, unlink, getpid):
     gsock = sock.UnixSocket('test.sock', mock.Mock(), mock.Mock(), fd)
     getpid.reset_mock()
     getpid.return_value = "fake"  # fake a pid change
-    gsock.destroy()
+    gsock.close(False)
     unlink.assert_not_called()