fix unix socket locking

This change add proper file locking to gunicorn. By default "gunicorn.lock" is created in the temporary directory when a unix socket is bound.  In case someone want to fix the lock file path or use multiple gunicorn instance the "--lock-file" setting can be used to set the path of this file.

fix #1259

gunicorn/arbiter.py

@@ -14,6 +14,7 @@
 import traceback
 
 from gunicorn.errors import HaltServer, AppImportError
+from gunicorn.lockfile import LockFile
 from gunicorn.pidfile import Pidfile
 from gunicorn.sock import create_sockets
 from gunicorn import util
@@ -39,6 +40,7 @@ class Arbiter(object):
     START_CTX = {}
 
     LISTENERS = []
+    LOCK_FILE = None
     WORKERS = {}
     PIPE = []
 
@@ -131,8 +133,16 @@ def start(self):
         self.cfg.on_starting(self)
 
         self.init_signals()
+        need_lock = False
         if not self.LISTENERS:
-            self.LISTENERS = create_sockets(self.cfg, self.log)
+            self.LISTENERS, need_lock = create_sockets(self.cfg, self.log)
+
+        self.log.info("lock file is needed %s", need_lock)
+        if need_lock:
+            self.log.info("%s created", self.cfg.lockfile)
+            if not self.LOCK_FILE:
+                self.LOCK_FILE = LockFile(self.cfg)
+            self.LOCK_FILE.lock()
 
         listeners_str = ",".join([str(l) for l in self.LISTENERS])
         self.log.debug("Arbiter booted")
@@ -335,8 +345,13 @@ def stop(self, graceful=True):
         :attr graceful: boolean, If True (the default) workers will be
         killed gracefully  (ie. trying to wait for the current connection)
         """
+        locked = False
+        if self.LOCK_FILE:
+            self.LOCK_FILE.unlock()
+            locked = self.LOCK_FILE.locked()
+
         for l in self.LISTENERS:
-            l.close()
+            l.close(locked)
         self.LISTENERS = []
         sig = signal.SIGTERM
         if not graceful:

gunicorn/config.py

@@ -904,6 +904,18 @@ class Pidfile(Setting):
         If not set, no PID file will be written.
         """
 
+class LockFile(Setting):
+    name = "lockfile"
+    section = "Server Mechanics"
+    cli = ["--lock-file"]
+    meta = "FILE"
+    validator = validate_string
+    default = util.tmpfile("gunicorn.lock")
+    desc = """\
+        A filename to use for the lock file. A lock file is created when using unix sockets.
+        If not set, the default temporary directory will be used to store it.
+        """
+
 class WorkerTmpDir(Setting):
     name = "worker_tmp_dir"
     section = "Server Mechanics"

gunicorn/lockfile.py

@@ -0,0 +1,74 @@
+# -*- coding: utf-8 -
+#
+# This file is part of gunicorn released under the MIT license.
+# See the NOTICE for more information.
+
+import os
+import platform
+import tempfile
+
+from gunicorn import util
+
+PLATFORM = platform.system()
+IS_CYGWIN = PLATFORM.startswith('CYGWIN')
+
+if os.name == 'nt':
+    import msvcrt
+
+    def _lock(fd):
+        msvcrt.locking(fd, msvcrt.LK_NBLCK, 1)
+
+
+    def _unlock(fd):
+        try:
+            msvcrt.locking(fd, msvcrt.LK_UNLCK, 1)
+        except OSError:
+            return False
+        return True
+
+else:
+    import fcntl
+
+    def _lock(fd):
+        fcntl.lockf(fd, fcntl.LOCK_SH | fcntl.LOCK_NB)
+
+
+    def _unlock(fd):
+        try:
+            fcntl.lockf(fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
+        except:
+            print("no unlock")
+            return False
+
+        return True
+
+
+class LockFile(object):
+    """Manage a LOCK file"""
+
+    def __init__(self, cfg):
+        self.fname = cfg.lockfile
+        fdir = os.path.dirname(self.fname)
+        if fdir and not os.path.isdir(fdir):
+            raise RuntimeError("%s doesn't exist. Can't create lock file." % fdir)
+        self._lockfile = open(self.fname, 'w+b')
+        # set permissions to -rw-r--r--
+        os.chmod(self.fname, 420)
+        self._locked = False
+
+    def lock(self):
+        _lock(self._lockfile.fileno())
+        self._locked = 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._locked = False
+
+    def locked(self):
+        return self._lockfile is not None and self._locked

gunicorn/sock.py

@@ -53,7 +53,7 @@ def set_options(self, sock, bound=False):
     def bind(self, sock):
         sock.bind(self.cfg_addr)
 
-    def close(self):
+    def close(self, locked=False):
         if self.sock is None:
             return
 
@@ -110,8 +110,6 @@ def __init__(self, addr, conf, log, fd=None):
                     raise ValueError("%r is not a socket" % addr)
         self.parent = os.getpid()
         super(UnixSocket, self).__init__(addr, conf, log, fd=fd)
-        # each arbiter grabs a shared lock on the unix socket.
-        fcntl.lockf(self.sock, fcntl.LOCK_SH | fcntl.LOCK_NB)
 
     def __str__(self):
         return "unix:%s" % self.cfg_addr
@@ -122,18 +120,10 @@ def bind(self, sock):
         util.chown(self.cfg_addr, self.conf.uid, self.conf.gid)
         os.umask(old_umask)
 
-
-    def close(self):
-        if self.parent == os.getpid():
-            # attempt to acquire an exclusive lock on the unix socket.
-            # if we're the only arbiter running, the lock will succeed, and
-            # we can safely rm the socket.
-            try:
-                fcntl.lockf(self.sock, fcntl.LOCK_EX | fcntl.LOCK_NB)
-            except:
-                pass
-            else:
-                os.unlink(self.cfg_addr)
+    def close(self, locked=False):
+        print(os.getpid())
+        if self.parent == os.getpid() and not locked:
+            os.unlink(self.cfg_addr)
         super(UnixSocket, self).close()
 
 
@@ -162,6 +152,7 @@ def create_sockets(conf, log):
     # gunicorn.
     # http://www.freedesktop.org/software/systemd/man/systemd.socket.html
     listeners = []
+    need_lock = False
     if ('LISTEN_PID' in os.environ
             and int(os.environ.get('LISTEN_PID')) == os.getpid()):
         for i in range(int(os.environ.get('LISTEN_FDS', 0))):
@@ -170,6 +161,7 @@ def create_sockets(conf, log):
                 sock = socket.fromfd(fd, socket.AF_UNIX, socket.SOCK_STREAM)
                 sockname = sock.getsockname()
                 if isinstance(sockname, str) and sockname.startswith('/'):
+                    need_lock = True
                     listeners.append(UnixSocket(sockname, conf, log, fd=fd))
                 elif len(sockname) == 2 and '.' in sockname[0]:
                     listeners.append(TCPSocket("%s:%s" % sockname, conf, log,
@@ -184,7 +176,7 @@ def create_sockets(conf, log):
         if listeners:
             log.debug('Socket activation sockets: %s',
                     ",".join([str(l) for l in listeners]))
-            return listeners
+            return listeners, need_lock
 
     # get it only once
     laddr = conf.address
@@ -205,18 +197,23 @@ def create_sockets(conf, log):
             addr = laddr[i]
             sock_type = _sock_type(addr)
 
+            if sock_type == UnixSocket:
+                need_lock = True
+
             try:
                 listeners.append(sock_type(addr, conf, log, fd=fd))
             except socket.error as e:
                 if e.args[0] == errno.ENOTCONN:
                     log.error("GUNICORN_FD should refer to an open socket.")
                 else:
                     raise
-        return listeners
+        return listeners, need_lock
 
     # no sockets is bound, first initialization of gunicorn in this env.
     for addr in laddr:
         sock_type = _sock_type(addr)
+        if sock_type == UnixSocket:
+            need_lock = True
 
         # If we fail to create a socket from GUNICORN_FD
         # we fall through and try and open the socket
@@ -244,4 +241,4 @@ def create_sockets(conf, log):
 
         listeners.append(sock)
 
-    return listeners
+    return listeners, need_lock

gunicorn/util.py

@@ -17,6 +17,7 @@
 import socket
 import stat
 import sys
+import tempfile
 import textwrap
 import time
 import traceback
@@ -546,3 +547,6 @@ def app(environ, start_response):
         return [msg]
 
     return app
+
+def tmpfile(fname):
+    return os.path.join(tempfile.gettempdir(), fname)

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

@@ -0,0 +1,19 @@
+import os
+
+from gunicorn.config import Config
+from gunicorn.lockfile import LockFile
+from gunicorn.util import tmpfile
+
+def test_lockfile():
+    cfg = Config()
+    lockname = tmpfile("gunicorn-tests.lock")
+    cfg.set("lockfile", lockname)
+
+    lock_file = LockFile(cfg)
+    assert lock_file.locked() == False
+    assert os.path.exists(lockname)
+    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

@@ -1,5 +1,3 @@
-import fcntl
-
 try:
     import unittest.mock as mock
 except ImportError:
@@ -8,49 +6,31 @@
 from gunicorn import sock
 
 
-@mock.patch('fcntl.lockf')
-@mock.patch('socket.fromfd')
-def test_unix_socket_init_lock(fromfd, lockf):
-    s = fromfd.return_value
-    sock.UnixSocket('test.sock', mock.Mock(), mock.Mock(), mock.Mock())
-    lockf.assert_called_with(s, fcntl.LOCK_SH | fcntl.LOCK_NB)
-
-
-@mock.patch('fcntl.lockf')
 @mock.patch('os.getpid')
 @mock.patch('os.unlink')
 @mock.patch('socket.fromfd')
-def test_unix_socket_close_delete_if_exlock(fromfd, unlink, getpid, lockf):
-    s = fromfd.return_value
+def test_unix_socket_close_delete_if_exlock(fromfd, unlink, getpid):
     gsock = sock.UnixSocket('test.sock', mock.Mock(), mock.Mock(), mock.Mock())
-    lockf.reset_mock()
-    gsock.close()
-    lockf.assert_called_with(s, fcntl.LOCK_EX | fcntl.LOCK_NB)
+    gsock.close(False)
     unlink.assert_called_with('test.sock')
 
 
-@mock.patch('fcntl.lockf')
 @mock.patch('os.getpid')
 @mock.patch('os.unlink')
 @mock.patch('socket.fromfd')
-def test_unix_socket_close_keep_if_no_exlock(fromfd, unlink, getpid, lockf):
-    s = fromfd.return_value
+def test_unix_socket_close_keep_if_no_exlock(fromfd, unlink, getpid):
     gsock = sock.UnixSocket('test.sock', mock.Mock(), mock.Mock(), mock.Mock())
-    lockf.reset_mock()
-    lockf.side_effect = IOError('locked')
-    gsock.close()
-    lockf.assert_called_with(s, fcntl.LOCK_EX | fcntl.LOCK_NB)
+    gsock.close(True)
     unlink.assert_not_called()
 
 
-@mock.patch('fcntl.lockf')
 @mock.patch('os.getpid')
+@mock.patch('os.unlink')
 @mock.patch('socket.fromfd')
-def test_unix_socket_not_deleted_by_worker(fromfd, getpid, lockf):
+def test_unix_socket_not_deleted_by_worker(fromfd, unlink, getpid):
     fd = mock.Mock()
-    gsock = sock.UnixSocket('name', mock.Mock(), mock.Mock(), fd)
-    lockf.reset_mock()
+    gsock = sock.UnixSocket('test.sock', mock.Mock(), mock.Mock(), fd)
     getpid.reset_mock()
-    getpid.return_value = mock.Mock()  # fake a pid change
-    gsock.close()
-    lockf.assert_not_called()
+    getpid.return_value = "fake"  # fake a pid change
+    gsock.close(False)
+    unlink.assert_not_called()