lockfile improvements

- rename LockFile.lock  to acquire
- rename LockFile.unlock to release
- move the lockfile management in sepate functions inside the arbiter
- remove the "closed" argument from the socket.close method and add a new "destroy" function that will be called whent  the socket can be unlinked (cal release)
- fix tests

gunicorn/arbiter.py

@@ -138,16 +138,7 @@ class Arbiter(object):
             self.LISTENERS, need_lock = create_sockets(self.cfg, self.log)
 
         if need_lock:
-            if not self.LOCK_FILE:
+            self.acquire_lockfile()
-                # 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")
@@ -350,17 +341,13 @@ class Arbiter(object):
         :attr graceful: boolean, If True (the default) workers will be
         killed gracefully  (ie. trying to wait for the current connection)
         """
-        locked = False
+        released = self.try_release_lockfile()
-        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(locked)
+            l.close()
+
+            if released:
+                l.destroy()
+
         self.LISTENERS = []
         sig = signal.SIGTERM
         if not graceful:
@@ -453,6 +440,36 @@ 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,24 +49,27 @@ class LockFile(object):
         self._lockfile = open(self.fname, 'w+b')
         # set permissions to -rw-r--r--
         os.chmod(self.fname, 420)
-        self._locked = False
+        self._released = True
 
-    def lock(self):
+    def acquire(self):
         _lock(self._lockfile.fileno())
-        self._locked = True
+        self._released = False
 
-    def unlock(self):
+    def release(self):
-        if not self.locked():
+        if self.released():
-            return
+            return True
 
         if _unlock(self._lockfile.fileno()):
             self._lockfile.close()
             util.unlink(self.fname)
             self._lockfile = None
-            self._locked = False
+            self._released = True
+            return True
 
-    def locked(self):
+        return False
-        return self._lockfile is not None and self._locked
+
+    def released(self):
+        return self._lockfile is None or self._released
 
     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, locked=False):
+    def close(self):
         if self.sock is None:
             return
 
@@ -64,6 +64,9 @@ class BaseSocket(object):
 
         self.sock = None
 
+    def destroy(self):
+        pass
+
 
 class TCPSocket(BaseSocket):
 
@@ -120,11 +123,13 @@ class UnixSocket(BaseSocket):
         util.chown(self.cfg_addr, self.conf.uid, self.conf.gid)
         os.umask(old_umask)
 
-    def close(self, locked=False):
+    def close(self):
-        if self.parent == os.getpid() and not locked:
-            os.unlink(self.cfg_addr)
         super(UnixSocket, self).close()
 
+    def destroy(self):
+        if self.parent == os.getpid():
+            os.unlink(self.cfg_addr)
+
 
 def _sock_type(addr):
     if isinstance(addr, tuple):

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(False)
+    listener1.close.assert_called_with()
-    listener2.close.assert_called_with(False)
+    listener2.close.assert_called_with()
 
 
 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.locked() == False
+    assert lock_file.released() == True
     assert os.path.exists(lockname)
-    lock_file.lock()
+    lock_file.acquire()
-    assert lock_file.locked() == True
+    assert lock_file.released() == False
-    lock_file.unlock()
+    lock_file.release()
-    assert lock_file.locked() == False
+    assert lock_file.released() == True
     assert os.path.exists(lockname) == False

tests/test_sock.py

@@ -11,19 +11,10 @@ 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.close(False)
+    gsock.destroy()
     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')
@@ -32,5 +23,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.close(False)
+    gsock.destroy()
     unlink.assert_not_called()