Rewrite Windows backend to use IOCP exclusively

azure-pipelines.yml

@@ -39,12 +39,24 @@ jobs:
 - job: 'Windows'
   pool:
     vmImage: 'vs2017-win2016'
-  timeoutInMinutes: 10
+  timeoutInMinutes: 20
   strategy:
     # Python version list:
     #   64-bit: https://www.nuget.org/packages/python/
     #   32-bit: https://www.nuget.org/packages/pythonx86/
     matrix:
+      # The LSP tests can be super slow for some reason - like
+      # sometimes it just randomly takes 5 minutes to run the LSP
+      # installer. So we put them at the top, so they can get started
+      # earlier.
+      "with IFS LSP, Python 3.7, 64 bit":
+        python.version: '3.7.2'
+        python.pkg: 'python'
+        lsp: 'http://www.proxifier.com/download/ProxifierSetup.exe'
+      "with non-IFS LSP, Python 3.7, 64 bit":
+        python.version: '3.7.2'
+        python.pkg: 'python'
+        lsp: 'http://download.pctools.com/mirror/updates/9.0.0.2308-SDavfree-lite_en.exe'
       "Python 3.5, 32 bit":
         python.version: '3.5.4'
         python.pkg: 'pythonx86'

ci.sh

@@ -12,6 +12,11 @@ else
     CODECOV_NAME="${TRAVIS_OS_NAME}-${TRAVIS_PYTHON_VERSION:-unknown}"
 fi
 
+# We always want to retry on failure, and we have to set --connect-timeout to
+# work around a curl bug:
+#   https://github.com/curl/curl/issues/4461
+CURL="curl --connect-timeout 5 --retry 5"
+
 ################################################################
 # Bootstrap python environment, if necessary
 ################################################################
@@ -47,12 +52,12 @@ fi
 
 if [ "$TRAVIS_OS_NAME" = "osx" ]; then
     CODECOV_NAME="osx_${MACPYTHON}"
-    curl -Lo macpython.pkg https://www.python.org/ftp/python/${MACPYTHON}/python-${MACPYTHON}-macosx10.6.pkg
+    $CURL -Lo macpython.pkg https://www.python.org/ftp/python/${MACPYTHON}/python-${MACPYTHON}-macosx10.6.pkg
     sudo installer -pkg macpython.pkg -target /
     ls /Library/Frameworks/Python.framework/Versions/*/bin/
     PYTHON_EXE=/Library/Frameworks/Python.framework/Versions/*/bin/python3
     # The pip in older MacPython releases doesn't support a new enough TLS
-    curl https://bootstrap.pypa.io/get-pip.py | sudo $PYTHON_EXE
+    $CURL https://bootstrap.pypa.io/get-pip.py | sudo $PYTHON_EXE
     sudo $PYTHON_EXE -m pip install virtualenv
     $PYTHON_EXE -m virtualenv testenv
     source testenv/bin/activate
@@ -62,7 +67,7 @@ fi
 
 if [ "$PYPY_NIGHTLY_BRANCH" != "" ]; then
     CODECOV_NAME="pypy_nightly_${PYPY_NIGHTLY_BRANCH}"
-    curl -fLo pypy.tar.bz2 http://buildbot.pypy.org/nightly/${PYPY_NIGHTLY_BRANCH}/pypy-c-jit-latest-linux64.tar.bz2
+    $CURL -fLo pypy.tar.bz2 http://buildbot.pypy.org/nightly/${PYPY_NIGHTLY_BRANCH}/pypy-c-jit-latest-linux64.tar.bz2
     if [ ! -s pypy.tar.bz2 ]; then
         # We know:
         # - curl succeeded (200 response code; -f means "exit with error if
@@ -115,12 +120,40 @@ else
     # Actual tests
     python -m pip install -r test-requirements.txt
 
+    # If we're testing with a LSP installed, then it might break network
+    # stuff, so wait until after we've finished setting everything else
+    # up.
+    if [ "$LSP" != "" ]; then
+        echo "Installing LSP from ${LSP}"
+        $CURL -o lsp-installer.exe "$LSP"
+        # Double-slashes are how you tell windows-bash that you want a single
+        # slash, and don't treat this as a unix-style filename that needs to
+        # be replaced by a windows-style filename.
+        # http://www.mingw.org/wiki/Posix_path_conversion
+        ./lsp-installer.exe //silent //norestart
+        echo "Waiting for LSP to appear in Winsock catalog"
+        while ! netsh winsock show catalog | grep "Layered Chain Entry"; do
+            sleep 1
+        done
+        netsh winsock show catalog
+    fi
+
     mkdir empty
     cd empty
 
     INSTALLDIR=$(python -c "import os, trio; print(os.path.dirname(trio.__file__))")
     cp ../setup.cfg $INSTALLDIR
-    pytest -W error -r a --junitxml=../test-results.xml --run-slow ${INSTALLDIR} --cov="$INSTALLDIR" --cov-config=../.coveragerc --verbose
+    if pytest -W error -r a --junitxml=../test-results.xml --run-slow ${INSTALLDIR} --cov="$INSTALLDIR" --cov-config=../.coveragerc --verbose; then
+        PASSED=true
+    else
+        PASSED=false
+    fi
+
+    # Remove the LSP again; again we want to do this ASAP to avoid
+    # accidentally breaking other stuff.
+    if [ "$LSP" != "" ]; then
+        netsh winsock reset
+    fi
 
     # Disable coverage on 3.8 until we run 3.8 on Windows CI too
     #   https://github.com/python-trio/trio/pull/784#issuecomment-446438407
@@ -143,9 +176,9 @@ else
         #   bash <(curl ...)
         # but azure is broken:
         #   https://developercommunity.visualstudio.com/content/problem/743824/bash-task-on-windows-suddenly-fails-with-bash-devf.html
-        # Also we have to set --connect-timeout to work around:
-        #   https://github.com/curl/curl/issues/4461
-        curl --connect-timeout 5 --retry 5 -o codecov.sh https://codecov.io/bash
+        $CURL -o codecov.sh https://codecov.io/bash
         bash codecov.sh -n "${CODECOV_NAME}" -F "$FLAG"
     fi
+
+    $PASSED
 fi

docs/source/reference-hazmat.rst

@@ -165,13 +165,27 @@ All environments provide the following functions:
 
 .. function:: notify_closing(obj)
 
-   Call this before closing a file descriptor or ``SOCKET`` handle
-   that another task might be waiting on. This will cause any
-   `wait_readable` or `wait_writable` calls to immediately raise
+   Call this before closing a file descriptor (on Unix) or socket (on
+   Windows). This will cause any `wait_readable` or `wait_writable`
+   calls on the given object to immediately wake up and raise
    `~trio.ClosedResourceError`.
 
    This doesn't actually close the object – you still have to do that
-   yourself afterwards.
+   yourself afterwards. Also, you want to be careful to make sure no
+   new tasks start waiting on the object in between when you call this
+   and when it's actually closed. So to close something properly, you
+   usually want to do these steps in order:
+
+   1. Explicitly mark the object as closed, so that any new attempts
+      to use it will abort before they start.
+   2. Call `notify_closing` to wake up any already-existing users.
+   3. Actually close the object.
+
+   It's also possible to do them in a different order if that's more
+   convenient, *but only if* you make sure not to have any checkpoints in
+   between the steps. This way they all happen in a single atomic
+   step, so other tasks won't be able to tell what order they happened
+   in anyway.
 
 
 Unix-specific API

newsfragments/52.feature.rst

@@ -0,0 +1,4 @@
+Trio's Windows backend was rewritten to use IOCP exclusively, instead
+of a hybrid of ``select`` + IOCP. This should make it much faster and
+more scalable. It also simplifies the code internally, and paves the
+way for future improvements.

trio/_core/_generated_io_windows.py

@@ -6,65 +6,65 @@
 
 
 
-def current_iocp():
+async def wait_readable(sock):
     locals()[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
     try:
-        return  GLOBAL_RUN_CONTEXT.runner.io_manager.current_iocp()
+        return await GLOBAL_RUN_CONTEXT.runner.io_manager.wait_readable(sock)
     except AttributeError:
         raise RuntimeError('must be called from async context')
 
-def register_with_iocp(handle):
+async def wait_writable(sock):
     locals()[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
     try:
-        return  GLOBAL_RUN_CONTEXT.runner.io_manager.register_with_iocp(handle)
+        return await GLOBAL_RUN_CONTEXT.runner.io_manager.wait_writable(sock)
     except AttributeError:
         raise RuntimeError('must be called from async context')
 
-async def wait_overlapped(handle, lpOverlapped):
+def notify_closing(handle):
     locals()[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
     try:
-        return await GLOBAL_RUN_CONTEXT.runner.io_manager.wait_overlapped(handle, lpOverlapped)
+        return  GLOBAL_RUN_CONTEXT.runner.io_manager.notify_closing(handle)
     except AttributeError:
         raise RuntimeError('must be called from async context')
 
-def monitor_completion_key():
+def register_with_iocp(handle):
     locals()[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
     try:
-        return  GLOBAL_RUN_CONTEXT.runner.io_manager.monitor_completion_key()
+        return  GLOBAL_RUN_CONTEXT.runner.io_manager.register_with_iocp(handle)
     except AttributeError:
         raise RuntimeError('must be called from async context')
 
-async def wait_readable(sock):
+async def wait_overlapped(handle, lpOverlapped):
     locals()[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
     try:
-        return await GLOBAL_RUN_CONTEXT.runner.io_manager.wait_readable(sock)
+        return await GLOBAL_RUN_CONTEXT.runner.io_manager.wait_overlapped(handle, lpOverlapped)
     except AttributeError:
         raise RuntimeError('must be called from async context')
 
-async def wait_writable(sock):
+async def write_overlapped(handle, data, file_offset=0):
     locals()[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
     try:
-        return await GLOBAL_RUN_CONTEXT.runner.io_manager.wait_writable(sock)
+        return await GLOBAL_RUN_CONTEXT.runner.io_manager.write_overlapped(handle, data, file_offset)
     except AttributeError:
         raise RuntimeError('must be called from async context')
 
-def notify_closing(sock):
+async def readinto_overlapped(handle, buffer, file_offset=0):
     locals()[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
     try:
-        return  GLOBAL_RUN_CONTEXT.runner.io_manager.notify_closing(sock)
+        return await GLOBAL_RUN_CONTEXT.runner.io_manager.readinto_overlapped(handle, buffer, file_offset)
     except AttributeError:
         raise RuntimeError('must be called from async context')
 
-async def write_overlapped(handle, data, file_offset=0):
+def current_iocp():
     locals()[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
     try:
-        return await GLOBAL_RUN_CONTEXT.runner.io_manager.write_overlapped(handle, data, file_offset)
+        return  GLOBAL_RUN_CONTEXT.runner.io_manager.current_iocp()
     except AttributeError:
         raise RuntimeError('must be called from async context')
 
-async def readinto_overlapped(handle, buffer, file_offset=0):
+def monitor_completion_key():
     locals()[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
     try:
-        return await GLOBAL_RUN_CONTEXT.runner.io_manager.readinto_overlapped(handle, buffer, file_offset)
+        return  GLOBAL_RUN_CONTEXT.runner.io_manager.monitor_completion_key()
     except AttributeError:
         raise RuntimeError('must be called from async context')