diff --git a/gunicorn/arbiter.py b/gunicorn/arbiter.py index 99a435ba..b64d2a01 100644 --- a/gunicorn/arbiter.py +++ b/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 diff --git a/gunicorn/lockfile.py b/gunicorn/lockfile.py index 6b841d9d..1815611a 100644 --- a/gunicorn/lockfile.py +++ b/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 diff --git a/gunicorn/sock.py b/gunicorn/sock.py index 67d20671..341a48c3 100644 --- a/gunicorn/sock.py +++ b/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): diff --git a/tests/test_arbiter.py b/tests/test_arbiter.py index d6e06dfd..277ae768 100644 --- a/tests/test_arbiter.py +++ b/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): diff --git a/tests/test_lockfile.py b/tests/test_lockfile.py index 9ca485cf..1e4d683d 100644 --- a/tests/test_lockfile.py +++ b/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 diff --git a/tests/test_sock.py b/tests/test_sock.py index 0782bf7d..92c2aeb0 100644 --- a/tests/test_sock.py +++ b/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()