Fix Ctrl+C and tests on Windows. (#1808)

* Fix Ctrl+C on Windows.

* Disable testing of a function N/A on Windows.

* Add test for coverage, avoid crash on missing _stopping.

* Initialise StreamingHTTPResponse.protocol = None

* Improved comments.

* Reduce amount of data in test_request_stream to avoid failures on Windows.

* The Windows test doesn't work on Windows :(

* Use port numbers more likely to be free than 8000.

* Disable the other signal tests on Windows as well.

* Windows doesn't properly support SO_REUSEADDR, so that's disabled in Python, and thus rebinding fails. For successful testing, reuse port instead.

* app.run argument handling: added server kwargs (alike create_server), added warning on extra kwargs, made auto_reload explicit argument. Another go at Windows tests

* Revert "app.run argument handling: added server kwargs (alike create_server), added warning on extra kwargs, made auto_reload explicit argument. Another go at Windows tests"

This reverts commit dc5d682448.

* Use random test server port on most tests. Should avoid port/addr reuse issues.

* Another test to random port instead of 8000.

* Fix deprecation warnings about missing name on Sanic() in tests.

* Linter and typing

* Increase test coverage

* Rewrite test for ctrlc_windows_workaround

* py36 compat

* py36 compat

* py36 compat

* Don't rely on loop internals but add a stopping flag to app.

* App may be restarted.

* py36 compat

* Linter

* Add a constant for OS checking.

Co-authored-by: L. Kärkkäinen <tronic@users.noreply.github.com>

sanic/app.py

@@ -81,6 +81,7 @@ class Sanic:
         self.sock = None
         self.strict_slashes = strict_slashes
         self.listeners = defaultdict(list)
+        self.is_stopping = False
         self.is_running = False
         self.is_request_stream = False
         self.websocket_enabled = False
@@ -1177,6 +1178,7 @@ class Sanic:
 
         try:
             self.is_running = True
+            self.is_stopping = False
             if workers > 1 and os.name != "posix":
                 logger.warn(
                     f"Multiprocessing is currently not supported on {os.name},"
@@ -1209,7 +1211,9 @@ class Sanic:
 
     def stop(self):
         """This kills the Sanic"""
-        get_event_loop().stop()
+        if not self.is_stopping:
+            self.is_stopping = True
+            get_event_loop().stop()
 
     async def create_server(
         self,

sanic/compat.py

@@ -1,3 +1,6 @@
+import asyncio
+import signal
+
 from sys import argv
 
 from multidict import CIMultiDict  # type: ignore
@@ -23,3 +26,27 @@ else:
 
     async def open_async(file, mode="r", **kwargs):
         return aio_open(file, mode, **kwargs)
+
+
+def ctrlc_workaround_for_windows(app):
+    async def stay_active(app):
+        """Asyncio wakeups to allow receiving SIGINT in Python"""
+        while not die:
+            # If someone else stopped the app, just exit
+            if app.is_stopping:
+                return
+            # Windows Python blocks signal handlers while the event loop is
+            # waiting for I/O. Frequent wakeups keep interrupts flowing.
+            await asyncio.sleep(0.1)
+        # Can't be called from signal handler, so call it from here
+        app.stop()
+
+    def ctrlc_handler(sig, frame):
+        nonlocal die
+        if die:
+            raise KeyboardInterrupt("Non-graceful Ctrl+C")
+        die = True
+
+    die = False
+    signal.signal(signal.SIGINT, ctrlc_handler)
+    app.add_task(stay_active)

sanic/response.py

@@ -91,6 +91,7 @@ class StreamingHTTPResponse(BaseHTTPResponse):
         self.headers = Header(headers or {})
         self.chunked = chunked
         self._cookies = None
+        self.protocol = None
 
     async def write(self, data):
         """Writes a chunk of data to the streaming response.

sanic/server.py

@@ -15,7 +15,7 @@ from time import time
 from httptools import HttpRequestParser  # type: ignore
 from httptools.parser.errors import HttpParserError  # type: ignore
 
-from sanic.compat import Header
+from sanic.compat import Header, ctrlc_workaround_for_windows
 from sanic.exceptions import (
     HeaderExpectationFailed,
     InvalidUsage,
@@ -37,6 +37,8 @@ try:
 except ImportError:
     pass
 
+OS_IS_WINDOWS = os.name == "nt"
+
 
 class Signal:
     stopped = False
@@ -929,15 +931,11 @@ def serve(
 
     # Register signals for graceful termination
     if register_sys_signals:
-        _singals = (SIGTERM,) if run_multiple else (SIGINT, SIGTERM)
-        for _signal in _singals:
-            try:
-                loop.add_signal_handler(_signal, loop.stop)
-            except NotImplementedError:
-                logger.warning(
-                    "Sanic tried to use loop.add_signal_handler "
-                    "but it is not implemented on this platform."
-                )
+        if OS_IS_WINDOWS:
+            ctrlc_workaround_for_windows(app)
+        else:
+            for _signal in [SIGTERM] if run_multiple else [SIGINT, SIGTERM]:
+                loop.add_signal_handler(_signal, app.stop)
     pid = os.getpid()
     try:
         logger.info("Starting worker [%s]", pid)

sanic/testing.py

@@ -12,7 +12,7 @@ from sanic.response import text
 
 ASGI_HOST = "mockserver"
 HOST = "127.0.0.1"
-PORT = 42101
+PORT = None
 
 
 class SanicTestClient:
@@ -95,7 +95,7 @@ class SanicTestClient:
 
         if self.port:
             server_kwargs = dict(
-                host=host or self.host, port=self.port, **server_kwargs
+                host=host or self.host, port=self.port, **server_kwargs,
             )
             host, port = host or self.host, self.port
         else:
@@ -103,6 +103,7 @@ class SanicTestClient:
             sock.bind((host or self.host, 0))
             server_kwargs = dict(sock=sock, **server_kwargs)
             host, port = sock.getsockname()
+            self.port = port
 
         if uri.startswith(
             ("http:", "https:", "ftp:", "ftps://", "//", "ws:", "wss:")
@@ -114,6 +115,9 @@ class SanicTestClient:
             url = "{scheme}://{host}:{port}{uri}".format(
                 scheme=scheme, host=host, port=port, uri=uri
             )
+        # Tests construct URLs using PORT = None, which means random port not
+        # known until this function is called, so fix that here
+        url = url.replace(":None/", f":{port}/")
 
         @self.app.listener("after_server_start")
         async def _collect_response(sanic, loop):
@@ -203,7 +207,7 @@ class SanicASGITestClient(httpx.AsyncClient):
 
         self.app = app
 
-        dispatch = SanicASGIDispatch(app=app, client=(ASGI_HOST, PORT))
+        dispatch = SanicASGIDispatch(app=app, client=(ASGI_HOST, PORT or 0))
         super().__init__(dispatch=dispatch, base_url=base_url)
 
         self.last_request = None