diff --git a/docs/design/companion-process-manager.md b/docs/design/companion-process-manager.md index 8c3c690b..2f8cbcd6 100644 --- a/docs/design/companion-process-manager.md +++ b/docs/design/companion-process-manager.md @@ -680,7 +680,7 @@ No per-companion logic in Arbiter. - [x] Implement `start_process`. - [x] Implement `stop_process`. - [x] Implement `restart_process`. -- [ ] Preserve and clear `manual_stop` correctly. +- [x] Preserve and clear `manual_stop` correctly. - [ ] Add Unix control socket. - [ ] Implement JSON command protocol. - [ ] Implement `status`. diff --git a/gunicorn/companion/manager.py b/gunicorn/companion/manager.py index 65ba365f..59281138 100644 --- a/gunicorn/companion/manager.py +++ b/gunicorn/companion/manager.py @@ -37,13 +37,17 @@ class CompanionManager: Parent records the pid and moves the companion to STARTING. Child resolves and runs the target, exiting the worker on any failure so a crashed companion never leaks back into the manager's control flow. + + Spawning is policy-neutral: it does not touch ``manual_stop``. Clearing + that flag is the job of the commands that intentionally bring a + companion back (:meth:`start_process`, :meth:`restart_process`), and a + companion only ever reaches a respawn path with the flag already false. """ pid = os.fork() if pid != 0: proc.pid = pid proc.state = State.STARTING proc.started_at = time.time() - proc.manual_stop = False self.log.info("companion %s started (pid %s)", proc.name, pid) return pid diff --git a/tests/test_companion_manager.py b/tests/test_companion_manager.py index 13247e55..cca1bfef 100644 --- a/tests/test_companion_manager.py +++ b/tests/test_companion_manager.py @@ -274,6 +274,40 @@ def test_restart_process_stopping_rejected(): assert not ok and "stopping" in msg +def test_manual_stop_preserved_through_exit(): + # stop a running companion, then reap its child: it must settle in STOPPED + # with manual_stop still set so it is not auto-restarted. + mgr = make_manager("rq") + proc = mgr.processes["rq"] + proc.state = State.RUNNING + proc.pid = 60 + with mock.patch("os.kill"): + mgr.stop_process("rq", now=10.0) + with mock.patch("os.waitpid", side_effect=[(60, 0), (0, 0)]), \ + mock.patch("os.fork") as fork: + mgr.reap_processes() + fork.assert_not_called() + assert proc.state == State.STOPPED and proc.manual_stop is True + + +def test_start_clears_manual_stop(): + mgr = make_manager("rq") + proc = mgr.processes["rq"] + proc.manual_stop = True + with mock.patch("os.fork", return_value=61): + mgr.start_process("rq") + assert proc.manual_stop is False + + +def test_spawn_does_not_touch_manual_stop(): + mgr = make_manager("rq") + proc = mgr.processes["rq"] + proc.manual_stop = True + with mock.patch("os.fork", return_value=62): + mgr.spawn_process(proc) + assert proc.manual_stop is True + + def test_handle_exit_unexpected_backoff(): mgr = make_manager("rq") proc = mgr.processes["rq"]