diff --git a/CHANGES.txt b/CHANGES.txt index c7f32ea4..f90eae6f 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -4,6 +4,14 @@ Unreleased Bugfix ~~~~~~ +- ``BaseWSGIServer.close()`` now shuts down the task dispatcher's worker + threads, matching ``MultiSocketServer.close()``'s existing behavior. This + matters most for the common single-listen-address case, where + ``create_server()`` returns a bare ``BaseWSGIServer`` (not wrapped in + ``MultiSocketServer``), so calling ``.close()`` directly used to leave the + dispatcher's worker threads running. See + https://github.com/Pylons/waitress/issues/480 + - Renamed the HTTP header "Trailers" to "Trailer" to fix a typo and comply with the correct header name as specified in RFC 7230. diff --git a/src/waitress/server.py b/src/waitress/server.py index c56530a0..73b40a10 100644 --- a/src/waitress/server.py +++ b/src/waitress/server.py @@ -355,6 +355,15 @@ def print_listen(self, format_str): # pragma: no cover def close(self): self.trigger.close() + # Ensure the task dispatcher's worker threads are stopped too, + # matching MultiSocketServer.close()'s existing behavior. This + # matters most for the common single-listen-address case, + # where create_server() returns a bare BaseWSGIServer (not + # wrapped in MultiSocketServer), so calling .close() on it is + # the only place a caller has to shut things down -- without + # this, the dispatcher's worker threads were left running + # after close(). See GH #480. + self.task_dispatcher.shutdown() return wasyncore.dispatcher.close(self) diff --git a/tests/test_server.py b/tests/test_server.py index cede49a7..da3c8d0b 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -124,6 +124,25 @@ def test_run(self): inst.run() self.assertTrue(inst.task_dispatcher.was_shutdown) + def test_close_shuts_down_task_dispatcher(self): + """ + Regression test for + https://github.com/Pylons/waitress/issues/480 + + close() must shut down the task dispatcher's worker threads + directly, not only as a side effect of run()'s + SystemExit/KeyboardInterrupt handling. This matters most for + the common single-listen-address case, where create_server() + returns a bare BaseWSGIServer (not wrapped in + MultiSocketServer), so calling .close() directly -- without + ever calling .run() -- is the only place a caller has to shut + things down. + """ + inst = self._makeOneWithMap(_start=False) + inst.task_dispatcher = DummyTaskDispatcher() + inst.close() + self.assertTrue(inst.task_dispatcher.was_shutdown) + def test_run_base_server(self): inst = self._makeOneWithMulti(_start=False) inst.asyncore = DummyAsyncore()