From 7392b68ccc2e091d353a3cba96c38d770121ec7e Mon Sep 17 00:00:00 2001 From: Paul Fisher Date: Mon, 11 Jan 2016 11:26:30 -0800 Subject: [PATCH 1/2] lock domain socket and remove on last arbiter exit --- gunicorn/sock.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/gunicorn/sock.py b/gunicorn/sock.py index 4de9728541..fdb5ae752e 100644 --- a/gunicorn/sock.py +++ b/gunicorn/sock.py @@ -4,6 +4,7 @@ # See the NOTICE for more information. import errno +import fcntl import os import socket import stat @@ -105,6 +106,8 @@ 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 @@ -117,9 +120,17 @@ def bind(self, sock): def close(self): - super(UnixSocket, self).close() if self.parent == os.getpid(): - os.unlink(self.cfg_addr) + # 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) + super(UnixSocket, self).close() def _sock_type(addr): From 45d63301e2e896787661c0192d70e3032ff7cf76 Mon Sep 17 00:00:00 2001 From: Randall Leeds Date: Sun, 13 Mar 2016 15:56:22 -0700 Subject: [PATCH 2/2] add tests for arbiter unix socket locking --- tests/test_sock.py | 56 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 tests/test_sock.py diff --git a/tests/test_sock.py b/tests/test_sock.py new file mode 100644 index 0000000000..aa094b9f78 --- /dev/null +++ b/tests/test_sock.py @@ -0,0 +1,56 @@ +import fcntl + +try: + import unittest.mock as mock +except ImportError: + import mock + +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 + 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) + 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 + 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) + unlink.assert_not_called() + + +@mock.patch('fcntl.lockf') +@mock.patch('os.getpid') +@mock.patch('socket.fromfd') +def test_unix_socket_not_deleted_by_worker(fromfd, getpid, lockf): + fd = mock.Mock() + gsock = sock.UnixSocket('name', mock.Mock(), mock.Mock(), fd) + lockf.reset_mock() + getpid.reset_mock() + getpid.return_value = mock.Mock() # fake a pid change + gsock.close() + lockf.assert_not_called()