Skip to content

Commit

Permalink
Fix double ctrl-c kill (#2634)
Browse files Browse the repository at this point in the history
  • Loading branch information
ahopkins committed Dec 18, 2022
1 parent f7040cc commit 4744a89
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 6 deletions.
8 changes: 4 additions & 4 deletions sanic/worker/manager.py
Expand Up @@ -40,7 +40,7 @@ def __init__(
self.monitor_publisher, self.monitor_subscriber = monitor_pubsub
self.worker_state = worker_state
self.worker_state[self.MAIN_IDENT] = {"pid": self.pid}
self.terminated = False
self._shutting_down = False
self._serve = serve
self._server_settings = server_settings
self._server_count = count()
Expand Down Expand Up @@ -116,10 +116,9 @@ def join(self):
self.join()

def terminate(self):
if not self.terminated:
if not self._shutting_down:
for process in self.processes:
process.terminate()
self.terminated = True

def restart(
self,
Expand Down Expand Up @@ -257,7 +256,7 @@ def kill(self):
raise ServerKilled

def shutdown_signal(self, signal, frame):
if self.terminated:
if self._shutting_down:
logger.info("Shutdown interrupted. Killing.")
with suppress(ServerKilled):
self.kill()
Expand All @@ -270,6 +269,7 @@ def shutdown(self):
for process in self.processes:
if process.is_alive():
process.terminate()
self._shutting_down = True

@property
def pid(self):
Expand Down
20 changes: 18 additions & 2 deletions tests/worker/test_manager.py
Expand Up @@ -33,9 +33,7 @@ def test_terminate(os_mock: Mock):
context = Mock()
context.Process.return_value = process
manager = WorkerManager(1, fake_serve, {}, context, (Mock(), Mock()), {})
assert manager.terminated is False
manager.terminate()
assert manager.terminated is True
os_mock.kill.assert_called_once_with(1234, SIGINT)


Expand Down Expand Up @@ -63,6 +61,24 @@ def test_kill(os_mock: Mock):
os_mock.kill.assert_called_once_with(1234, SIGKILL)


@patch("sanic.worker.process.os")
@patch("sanic.worker.manager.os")
def test_shutdown_signal_send_kill(
manager_os_mock: Mock, process_os_mock: Mock
):
process = Mock()
process.pid = 1234
context = Mock()
context.Process.return_value = process
manager = WorkerManager(1, fake_serve, {}, context, (Mock(), Mock()), {})
assert manager._shutting_down is False
manager.shutdown_signal(SIGINT, None)
assert manager._shutting_down is True
process_os_mock.kill.assert_called_once_with(1234, SIGINT)
manager.shutdown_signal(SIGINT, None)
manager_os_mock.kill.assert_called_once_with(1234, SIGKILL)


def test_restart_all():
p1 = Mock()
p2 = Mock()
Expand Down

0 comments on commit 4744a89

Please sign in to comment.