From 87c5f3f942a1744256c89afa1c2c848a2975ea2d Mon Sep 17 00:00:00 2001 From: John Belmonte Date: Sun, 27 Jan 2019 21:44:16 +0900 Subject: [PATCH 1/2] proof of concept for deterministic close race --- tests/issue_96.py | 5 +++-- trio_websocket/_impl.py | 5 +++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/issue_96.py b/tests/issue_96.py index d73eccd..1b74486 100644 --- a/tests/issue_96.py +++ b/tests/issue_96.py @@ -81,7 +81,7 @@ async def handler(request): try: ws = await request.accept() await ws.send_message('issue96') - await trio.sleep(random()/1000) + await ws._for_testing_peer_closed_connection.wait() await trio.aclose_forcefully(ws._stream) except ConnectionClosed: pass @@ -93,13 +93,14 @@ async def main(): ssl_context=None) await nursery.start(serve) - for n in range(1000): + for n in range(1): logging.info('Connection %d', n) try: connection = await connect_websocket(nursery, 'localhost', 8000, '/', use_ssl=False) await connection.get_message() await connection.aclose() + await trio.sleep(.1) except ConnectionClosed: pass diff --git a/trio_websocket/_impl.py b/trio_websocket/_impl.py index 1d4fb51..fc9de8a 100644 --- a/trio_websocket/_impl.py +++ b/trio_websocket/_impl.py @@ -544,6 +544,9 @@ def __init__(self, stream, wsproto, *, path=None, # Set once a WebSocket closed handshake takes place, i.e after a close # frame has been sent and a close frame has been received. self._close_handshake = trio.Event() + # Set immediately upon receiving closed event from peer. Used to + # test close race conditions between client and server. + self._for_testing_peer_closed_connection = trio.Event() @property def closed(self): @@ -807,6 +810,8 @@ async def _handle_connection_closed_event(self, event): :param event: ''' + self._for_testing_peer_closed_connection.set() + await trio.sleep(0) await self._write_pending() await self._close_web_socket(event.code, event.reason or None) self._close_handshake.set() From 3ae258776cea7c81ab676432f15675bf03fcc554 Mon Sep 17 00:00:00 2001 From: John Belmonte Date: Thu, 14 Feb 2019 21:15:29 +0900 Subject: [PATCH 2/2] retire offline test script in favor of first-class test case --- tests/issue_96.py | 111 --------------------------------------- tests/test_connection.py | 20 +++++++ 2 files changed, 20 insertions(+), 111 deletions(-) delete mode 100644 tests/issue_96.py diff --git a/tests/issue_96.py b/tests/issue_96.py deleted file mode 100644 index 1b74486..0000000 --- a/tests/issue_96.py +++ /dev/null @@ -1,111 +0,0 @@ -''' -This is a test for issue #96: -https://github.com/HyperionGray/trio-websocket/issues/96 - -This is not a part of the automated tests, because its prone to a race condition -and I have not been able to trigger it deterministically. Instead, this "test" -connects multiple times, triggering the bug on average once or twice out of -every 10 connections. The bug causes an uncaught exception which causes the -script to crash. - -Note that the server handler and the client have `try/except ConnectionClosed` -blocks. The bug in issue #96 is that the exception is raised in a background -task, so it is not caught by the caller's try/except block. Instead, it crashes -the nursery that the background task is running in. - -Here's an example run where the test fails: - -(venv) $ python tests/issue_96.py -INFO:root:Connection 0 -INFO:root:Connection 1 -INFO:root:Connection 2 -INFO:root:Connection 3 -INFO:root:Connection 4 -INFO:root:Connection 5 -INFO:root:Connection 6 -INFO:root:Connection 7 -INFO:root:Connection 8 -INFO:root:Connection 9 -INFO:root:Connection 10 -INFO:root:Connection 11 -Traceback (most recent call last): - File "tests/issue_96.py", line 58, in - trio.run(main) - File "/home/mhaase/code/trio-websocket/venv/lib/python3.6/site-packages/trio/_core/_run.py", line 1337, in run - raise runner.main_task_outcome.error - File "tests/issue_96.py", line 54, in main - nursery.cancel_scope.cancel() - File "/home/mhaase/code/trio-websocket/venv/lib/python3.6/site-packages/trio/_core/_run.py", line 397, in __aexit__ - raise combined_error_from_nursery - File "/home/mhaase/code/trio-websocket/trio_websocket/_impl.py", line 327, in serve_websocket - await server.run(task_status=task_status) - File "/home/mhaase/code/trio-websocket/trio_websocket/_impl.py", line 1097, in run - await trio.sleep_forever() - File "/home/mhaase/code/trio-websocket/venv/lib/python3.6/site-packages/trio/_core/_run.py", line 397, in __aexit__ - raise combined_error_from_nursery - File "/home/mhaase/code/trio-websocket/venv/lib/python3.6/site-packages/trio/_highlevel_serve_listeners.py", line 129, in serve_listeners - task_status.started(listeners) - File "/home/mhaase/code/trio-websocket/venv/lib/python3.6/site-packages/trio/_core/_run.py", line 397, in __aexit__ - raise combined_error_from_nursery - File "/home/mhaase/code/trio-websocket/venv/lib/python3.6/site-packages/trio/_highlevel_serve_listeners.py", line 27, in _run_handler - await handler(stream) - File "/home/mhaase/code/trio-websocket/trio_websocket/_impl.py", line 1125, in _handle_connection - await connection.aclose() - File "/home/mhaase/code/trio-websocket/venv/lib/python3.6/site-packages/trio/_core/_run.py", line 397, in __aexit__ - raise combined_error_from_nursery - File "/home/mhaase/code/trio-websocket/trio_websocket/_impl.py", line 927, in _reader_task - await handler(event) - File "/home/mhaase/code/trio-websocket/trio_websocket/_impl.py", line 811, in _handle_connection_closed_event - await self._write_pending() - File "/home/mhaase/code/trio-websocket/trio_websocket/_impl.py", line 968, in _write_pending - raise ConnectionClosed(self._close_reason) from None -trio_websocket._impl.ConnectionClosed: - -You may need to modify the sleep duration to reliably trigger the bug on your -system. On a successful test, it will make 1,000 connections without any -uncaught exceptions. -''' -import functools -import logging -from random import random - -import trio -from trio_websocket import connect_websocket, ConnectionClosed, serve_websocket - - -logging.basicConfig(level=logging.INFO) - - -async def handler(request): - ''' Reverse incoming websocket messages and send them back. ''' - try: - ws = await request.accept() - await ws.send_message('issue96') - await ws._for_testing_peer_closed_connection.wait() - await trio.aclose_forcefully(ws._stream) - except ConnectionClosed: - pass - - -async def main(): - async with trio.open_nursery() as nursery: - serve = functools.partial(serve_websocket, handler, 'localhost', 8000, - ssl_context=None) - await nursery.start(serve) - - for n in range(1): - logging.info('Connection %d', n) - try: - connection = await connect_websocket(nursery, 'localhost', 8000, - '/', use_ssl=False) - await connection.get_message() - await connection.aclose() - await trio.sleep(.1) - except ConnectionClosed: - pass - - nursery.cancel_scope.cancel() - - -if __name__ == '__main__': - trio.run(main) diff --git a/tests/test_connection.py b/tests/test_connection.py index 0ef3153..3a8727b 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -664,3 +664,23 @@ async def handler(request): await client.get_message() assert client.closed assert client.closed.code == 1009 + + +async def test_close_race(nursery, autojump_clock): + """server attempts close just as client disconnects (issue #96)""" + + async def handler(request): + ws = await request.accept() + await ws.send_message('foo') + await ws._for_testing_peer_closed_connection.wait() + # with bug, this would raise ConnectionClosed from websocket internal task + await trio.aclose_forcefully(ws._stream) + + server = await nursery.start( + partial(serve_websocket, handler, HOST, 0, ssl_context=None)) + + connection = await connect_websocket(nursery, HOST, server.port, + RESOURCE, use_ssl=False) + await connection.get_message() + await connection.aclose() + await trio.sleep(.1)