From 24dfa3c1d6b21e731bd167a13153968bba8fa5ce Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 26 Jan 2015 22:30:28 +0100 Subject: [PATCH 1/2] Issue #23095, asyncio: Fix _WaitHandleFuture.cancel() If UnregisterWaitEx() fais with ERROR_IO_PENDING, it doesn't mean that the wait is unregistered yet. We still have to wait until the wait is cancelled. --- Lib/asyncio/windows_events.py | 37 ++++++++++++++++------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/Lib/asyncio/windows_events.py b/Lib/asyncio/windows_events.py index c9ba7850ee1..8f1d9d2a128 100644 --- a/Lib/asyncio/windows_events.py +++ b/Lib/asyncio/windows_events.py @@ -126,14 +126,12 @@ class _BaseWaitHandleFuture(futures.Future): return self._registered = False + wait_handle = self._wait_handle + self._wait_handle = None try: - _overlapped.UnregisterWait(self._wait_handle) + _overlapped.UnregisterWait(wait_handle) except OSError as exc: - self._wait_handle = None - if exc.winerror == _overlapped.ERROR_IO_PENDING: - # ERROR_IO_PENDING is not an error, the wait was unregistered - self._unregister_wait_cb(None) - elif exc.winerror != _overlapped.ERROR_IO_PENDING: + if exc.winerror != _overlapped.ERROR_IO_PENDING: context = { 'message': 'Failed to unregister the wait handle', 'exception': exc, @@ -142,9 +140,10 @@ class _BaseWaitHandleFuture(futures.Future): if self._source_traceback: context['source_traceback'] = self._source_traceback self._loop.call_exception_handler(context) - else: - self._wait_handle = None - self._unregister_wait_cb(None) + return + # ERROR_IO_PENDING means that the unregister is pending + + self._unregister_wait_cb(None) def cancel(self): self._unregister_wait() @@ -209,14 +208,12 @@ class _WaitHandleFuture(_BaseWaitHandleFuture): return self._registered = False + wait_handle = self._wait_handle + self._wait_handle = None try: - _overlapped.UnregisterWaitEx(self._wait_handle, self._event) + _overlapped.UnregisterWaitEx(wait_handle, self._event) except OSError as exc: - self._wait_handle = None - if exc.winerror == _overlapped.ERROR_IO_PENDING: - # ERROR_IO_PENDING is not an error, the wait was unregistered - self._unregister_wait_cb(None) - elif exc.winerror != _overlapped.ERROR_IO_PENDING: + if exc.winerror != _overlapped.ERROR_IO_PENDING: context = { 'message': 'Failed to unregister the wait handle', 'exception': exc, @@ -225,11 +222,11 @@ class _WaitHandleFuture(_BaseWaitHandleFuture): if self._source_traceback: context['source_traceback'] = self._source_traceback self._loop.call_exception_handler(context) - else: - self._wait_handle = None - self._event_fut = self._proactor._wait_cancel( - self._event, - self._unregister_wait_cb) + return + # ERROR_IO_PENDING is not an error, the wait was unregistered + + self._event_fut = self._proactor._wait_cancel(self._event, + self._unregister_wait_cb) class PipeServer(object): From 41063d2a59a24e257cd9ce62137e36c862e3ab1e Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 26 Jan 2015 22:30:49 +0100 Subject: [PATCH 2/2] asyncio, Tulip issue 204: Fix IocpProactor.recv() If ReadFile() fails with ERROR_BROKEN_PIPE, the operation is not pending: don't register the overlapped. I don't know if WSARecv() can fail with ERROR_BROKEN_PIPE. Since Overlapped.WSARecv() already handled ERROR_BROKEN_PIPE, let me guess that it has the same behaviour than ReadFile(). --- Lib/asyncio/windows_events.py | 20 +++++++++++++------- Modules/overlapped.c | 4 ++-- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/Lib/asyncio/windows_events.py b/Lib/asyncio/windows_events.py index 8f1d9d2a128..94aafb6f5ab 100644 --- a/Lib/asyncio/windows_events.py +++ b/Lib/asyncio/windows_events.py @@ -406,13 +406,21 @@ class IocpProactor: self._results = [] return tmp + def _result(self, value): + fut = futures.Future(loop=self._loop) + fut.set_result(value) + return fut + def recv(self, conn, nbytes, flags=0): self._register_with_iocp(conn) ov = _overlapped.Overlapped(NULL) - if isinstance(conn, socket.socket): - ov.WSARecv(conn.fileno(), nbytes, flags) - else: - ov.ReadFile(conn.fileno(), nbytes) + try: + if isinstance(conn, socket.socket): + ov.WSARecv(conn.fileno(), nbytes, flags) + else: + ov.ReadFile(conn.fileno(), nbytes) + except BrokenPipeError: + return self._result(b'') def finish_recv(trans, key, ov): try: @@ -505,9 +513,7 @@ class IocpProactor: # ConnectNamePipe() failed with ERROR_PIPE_CONNECTED which means # that the pipe is connected. There is no need to wait for the # completion of the connection. - f = futures.Future(loop=self._loop) - f.set_result(pipe) - return f + return self._result(pipe) def finish_accept_pipe(trans, key, ov): ov.getresult() diff --git a/Modules/overlapped.c b/Modules/overlapped.c index 4661152d20b..1a081ecb770 100644 --- a/Modules/overlapped.c +++ b/Modules/overlapped.c @@ -730,7 +730,7 @@ Overlapped_ReadFile(OverlappedObject *self, PyObject *args) switch (err) { case ERROR_BROKEN_PIPE: mark_as_completed(&self->overlapped); - Py_RETURN_NONE; + return SetFromWindowsErr(err); case ERROR_SUCCESS: case ERROR_MORE_DATA: case ERROR_IO_PENDING: @@ -789,7 +789,7 @@ Overlapped_WSARecv(OverlappedObject *self, PyObject *args) switch (err) { case ERROR_BROKEN_PIPE: mark_as_completed(&self->overlapped); - Py_RETURN_NONE; + return SetFromWindowsErr(err); case ERROR_SUCCESS: case ERROR_MORE_DATA: case ERROR_IO_PENDING: