Add support for Windows platform

.github/workflows/test.yaml

@@ -25,16 +25,12 @@ on:
 
 jobs:
   pytest:
-    runs-on: ubuntu-22.04
     strategy:
       fail-fast: false
       matrix:
-        include:
-          - python-version: "3.8"
-          - python-version: "3.9"
-          - python-version: "3.10"
-          - python-version: "3.11"
-
+        os: [ubuntu-22.04, windows-2022]
+        python-version: ["3.8", "3.9", "3.10", "3.11"]
+    runs-on: ${{ matrix.os }}
     steps:
       - uses: actions/checkout@v3
       - uses: actions/setup-python@v4

pyproject.toml

@@ -47,6 +47,7 @@ Source = "https://github.com/jupyterhub/simpervisor"
 [project.optional-dependencies]
 test = [
     "aiohttp",
+    "psutil",
     "pytest",
     "pytest-asyncio",
     "pytest-cov",

simpervisor/atexitasync.py

@@ -3,7 +3,6 @@
 
 Handles SIGINT and SIGTERM, unlike atexit
 """
-import asyncio
 import signal
 import sys
 
@@ -15,9 +14,8 @@
 def add_handler(handler):
     global signal_handler_set
     if not signal_handler_set:
-        loop = asyncio.get_event_loop()
-        loop.add_signal_handler(signal.SIGINT, _handle_signal, signal.SIGINT)
-        loop.add_signal_handler(signal.SIGTERM, _handle_signal, signal.SIGTERM)
+        signal.signal(signal.SIGINT, _handle_signal)
+        signal.signal(signal.SIGTERM, _handle_signal)
         signal_handler_set = True
     _handlers.append(handler)
 
@@ -26,7 +24,11 @@ def remove_handler(handler):
     _handlers.remove(handler)
 
 
-def _handle_signal(signum):
+def _handle_signal(signum, *args):
+    # Windows doesn't support SIGINT. Replacing it with CTRL_C_EVENT so that it
+    # can used with subprocess.Popen.send_signal
+    if signum == signal.SIGINT and sys.platform == "win32":
+        signum = signal.CTRL_C_EVENT
     for handler in _handlers:
         handler(signum)
     sys.exit(0)

simpervisor/process.py

@@ -4,6 +4,8 @@
 import asyncio
 import logging
 import signal
+import subprocess
+import sys
 import time
 
 from .atexitasync import add_handler, remove_handler
@@ -17,6 +19,121 @@ class KilledProcessError(Exception):
     """
 
 
+class Process:
+    """
+    Abstract class to start, wait and send signals to running processes in a OS agnostic way
+    """
+
+    # Protected data members only accessible by derived classes.
+    _proc_cmd = None
+    _proc_kwargs = None
+    _proc = None
+
+    def __init__(self, *cmd, **kwargs):
+        self._proc_cmd = cmd
+        self._proc_kwargs = kwargs
+
+    async def start(self):
+        """
+        Start the process
+        """
+        raise NotImplementedError
+
+    async def wait(self):
+        """
+        Wait for the process to terminate and return the process exit code.
+        """
+        raise NotImplementedError
+
+    def get_kill_signal(self):
+        """
+        Returns the preferred OS signal to kill the process.
+        """
+        raise NotImplementedError
+
+    def send_signal(self, signum):
+        """
+        Send the OS signal to the process.
+        """
+        if self._proc:
+            self._proc.send_signal(signum)
+
+    @property
+    def pid(self):
+        if self._proc:
+            return self._proc.pid
+
+    @property
+    def returncode(self):
+        if self._proc:
+            return self._proc.returncode
+
+
+class POSIXProcess(Process):
+    """
+    A process that uses asyncio-subprocess API to start and wait.
+    """
+
+    async def start(self):
+        """
+        Start the process using asyncio.create_subprocess_exec API
+        """
+        self._proc = await asyncio.create_subprocess_exec(
+            *self._proc_cmd, **self._proc_kwargs
+        )
+
+    async def wait(self):
+        """
+        Wait for the process to stop and return the process exit code.
+        """
+        return await self._proc.wait()
+
+    def get_kill_signal(self):
+        """
+        Returns the OS signal used for kill the child process.
+        """
+        return signal.SIGKILL
+
+
+class WindowsProcess(Process):
+    """
+    A process that uses subprocess API to start and wait (uses busy polling).
+    """
+
+    async def start(self):
+        """
+        Starts the process using subprocess.Popen API
+        """
+        self._proc = subprocess.Popen(list(self._proc_cmd), **self._proc_kwargs)
+
+    async def wait(self):
+        """
+        Wait for the process to stop and return the process exit code.
+
+        subprocess.Popen.wait() is a blocking call which can cause the asyncio
+        event loop to remain blocked until the child process is terminated.
+
+        To circumvent this behavior, we use busy polling with asyncio.sleep to check
+        whether the child process is alive or not and keeping the asyncio event
+        loop running.
+
+        See https://github.com/jupyter/jupyter_client/blob/main/jupyter_client/provisioning/local_provisioner.py#L54_L55 for similar use.
+        """
+        while self._proc.poll() is None:
+            await asyncio.sleep(0.1)
+        return self._proc.wait()
+
+    def get_kill_signal(self):
+        """
+        Returns the OS signal used for kill the child process.
+
+        Windows doesn't support SIGKILL. subprocess.Popen.kill() is an alias of
+        subprocess.Popen.terminate(), so we can use SIGTERM instead of SIGKILL
+        on windows platform.
+        """
+        return signal.SIGTERM
+
+
 class SupervisedProcess:
     def __init__(
         self,
@@ -100,9 +217,15 @@ async def start(self):
                     f"Process {self.name} has already been explicitly killed"
                 )
             self._debug_log("try-start", "Trying to start {}", {}, self.name)
-            self.proc = await asyncio.create_subprocess_exec(
-                *self._proc_args, **self._proc_kwargs
-            )
+
+            # Child process is created based on platform
+            if sys.platform == "win32":
+                self.proc = WindowsProcess(*self._proc_args, **self._proc_kwargs)
+            else:
+                self.proc = POSIXProcess(*self._proc_args, **self._proc_kwargs)
+
+            # Start the child process
+            await self.proc.start()
             self._debug_log("started", "Started {}", {}, self.name)
 
             self._killed = False
@@ -180,7 +303,8 @@ async def kill(self):
             raise KilledProcessError(
                 f"Process {self.name} has already been explicitly killed"
             )
-        return await self._signal_and_wait(signal.SIGKILL)
+        signum = self.proc.get_kill_signal()
+        return await self._signal_and_wait(signum)
 
     async def ready(self):
         """
@@ -208,7 +332,9 @@ async def ready(self):
             # FIXME: We should probably check again if our process is still running
             # FIXME: Should we be locking something here?
             try:
-                is_ready = await asyncio.wait_for(self.ready_func(self), 1)
+                # Timeout of 5 secs is needed as DNS resolution of localhost
+                # on Windows takes significant time.
+                is_ready = await asyncio.wait_for(self.ready_func(self), 5)
             except asyncio.TimeoutError:
                 is_ready = False
             cur_time = time.time() - start_time

tests/test_atexitasync.py

@@ -16,6 +16,10 @@
         (signal.SIGINT, 5),
     ],
 )
+@pytest.mark.skipif(
+    sys.platform == "win32",
+    reason="Testing signals on Windows doesn't seem to be possible",
+)
 def test_atexitasync(signum, handlercount):
     """
     Test signal handlers receive signals properly

tests/test_ready.py

@@ -33,13 +33,19 @@ async def _ready_func(p):
                 logging.debug("Connection to {} refused", url)
                 return False
 
+    # If env variable is specified to SupervisedProcess, then it needs to be a
+    # copy of OS environment as interpreter related required information is
+    # stored on Windows.
+    env = os.environ.copy()
+    env.update({"PORT": port})
+
     proc = SupervisedProcess(
         "socketserver",
         sys.executable,
         httpserver_file,
         str(ready_time),
         ready_func=_ready_func,
-        env={"PORT": port},
+        env=env,
     )
 
     try:

tests/test_signals.py

@@ -9,6 +9,10 @@
 
 
 @pytest.mark.parametrize("childcount", [1, 5])
+@pytest.mark.skipif(
+    sys.platform == "win32",
+    reason="Testing signals on Windows doesn't seem to be possible",
+)
 async def test_sigtermreap(childcount):
     """
     Test reaping subprocess after SIGTERM.

tests/test_simpervisor.py

@@ -1,16 +1,19 @@
 import asyncio
-import errno
 import inspect
-import os
 import signal
 import sys
 
+import psutil
 import pytest
 
 from simpervisor import KilledProcessError, SupervisedProcess
 
 SLEEP_TIME = 0.1
-SLEEP_WAIT_TIME = 0.5
+
+# On Windows, for test_start_success testpoint, the process exit is taking more
+# than 0.5 seconds. Hence, to be on safe side setting the wait timeout value to
+# be 5 seconds.
+SLEEP_WAIT_TIME = 5
 
 
 def sleep(retcode=0, time=SLEEP_TIME):
@@ -122,11 +125,10 @@ async def test_kill():
 
     await proc.start()
     await proc.kill()
-    assert proc.returncode == -signal.SIGKILL
+    exitcode = 1 if sys.platform == "win32" else -signal.SIGKILL
+    assert proc.returncode == exitcode
     assert not proc.running
-    with pytest.raises(OSError) as e:
-        os.kill(proc.pid, 0)
-    assert e.value.errno == errno.ESRCH
+    assert not psutil.pid_exists(proc.pid)
 
 
 async def test_terminate():
@@ -139,8 +141,7 @@ async def test_terminate():
 
     await proc.start()
     await proc.terminate()
-    assert proc.returncode == -signal.SIGTERM
+    exitcode = 1 if sys.platform == "win32" else -signal.SIGTERM
+    assert proc.returncode == exitcode
     assert not proc.running
-    with pytest.raises(OSError) as e:
-        os.kill(proc.pid, 0)
-    assert e.value.errno == errno.ESRCH
+    assert not psutil.pid_exists(proc.pid)