diff --git a/CHANGES/12879.bugfix.rst b/CHANGES/12879.bugfix.rst new file mode 100644 index 00000000000..9dc8057a64d --- /dev/null +++ b/CHANGES/12879.bugfix.rst @@ -0,0 +1 @@ +Fixed ``GunicornWebWorker`` endlessly reloading when app fails during startup -- by :user:`Dreamsorcerer`. diff --git a/aiohttp/worker.py b/aiohttp/worker.py index a86573b1d45..6bbdbdcfc13 100644 --- a/aiohttp/worker.py +++ b/aiohttp/worker.py @@ -60,14 +60,20 @@ def init_process(self) -> None: super().init_process() def run(self) -> None: - self._task = self.loop.create_task(self._run()) + # base.Worker.init_process() sets self.booted = True before + # invoking run(), but for the aiohttp worker the real boot work + # (factory call, runner setup, binding sockets) happens here. + # Reset until _run() reaches the serve loop so that the arbiter + # can tell a startup failure from a normal worker exit and + # halt instead of endlessly respawning workers. + self.booted = False - try: # ignore all finalization problems + self._task = self.loop.create_task(self._run()) + try: self.loop.run_until_complete(self._task) - except Exception: - self.log.exception("Exception in gunicorn worker") - self.loop.run_until_complete(self.loop.shutdown_asyncgens()) - self.loop.close() + finally: + self.loop.run_until_complete(self.loop.shutdown_asyncgens()) + self.loop.close() sys.exit(self.exit_code) @@ -118,6 +124,12 @@ async def _run(self) -> None: ) await site.start() + # Sockets are bound; tell the arbiter the worker is ready to + # accept requests. Any failure before this point propagates out + # of run() with self.booted=False so the arbiter exits with + # WORKER_BOOT_ERROR instead of treating this as a clean exit. + self.booted = True + # If our parent changed then we shut down. pid = os.getpid() try: diff --git a/tests/test_worker.py b/tests/test_worker.py index 9be9e41c20c..9609d7b9d68 100644 --- a/tests/test_worker.py +++ b/tests/test_worker.py @@ -143,9 +143,33 @@ def test_run_not_app( worker.loop = loop worker.wsgi = "not-app" worker.alive = False - with pytest.raises(SystemExit): + with pytest.raises(RuntimeError, match="wsgi app should be"): + worker.run() + assert not worker.booted + assert loop.is_closed() + + +def test_run_on_startup_raises( + worker: base_worker.GunicornWebWorker, loop: asyncio.AbstractEventLoop +) -> None: + worker.log = mock.Mock() + worker.cfg = mock.Mock() + worker.cfg.access_log_format = ACCEPTABLE_LOG_FORMAT + worker.cfg.is_ssl = False + worker.cfg.graceful_timeout = 100 + worker.sockets = [] + + app = web.Application() + + async def boom(app: web.Application) -> None: + raise RuntimeError("boom during startup") + + app.on_startup.append(boom) + worker.wsgi = app + worker.loop = loop + with pytest.raises(RuntimeError, match="boom during startup"): worker.run() - worker.log.exception.assert_called_with("Exception in gunicorn worker") + assert not worker.booted assert loop.is_closed()