Skip to content

Commit 77a712a

Browse files
authored
fix reason not being set after local close (#159)
During a local-initiated close handshake, the following incorrect behavior was observed: * `closed` attribute would be `None` * `send_message()` would be silently ignored (wsproto < 0.2.0) or leak a `LocalProtocolException` (wsproto >= 0.2.0) Upon local-initiated close, `closed` will now have the reason, and `send_message()` will raise `ConnectionClosed`. Fixes #158.
1 parent 016bebb commit 77a712a

File tree

2 files changed

+37
-3
lines changed

2 files changed

+37
-3
lines changed

tests/test_connection.py

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -791,7 +791,7 @@ async def handler(request):
791791
server = await nursery.start(
792792
partial(serve_websocket, handler, HOST, 0, ssl_context=None))
793793

794-
# connection should close when server handler exists
794+
# connection should close when server handler exits
795795
with trio.fail_after(2):
796796
async with open_websocket(
797797
HOST, server.port, '/', use_ssl=False) as connection:
@@ -955,6 +955,35 @@ async def handler(request: WebSocketRequest):
955955
await client.send_message('bar')
956956

957957

958+
async def test_message_after_local_close_race(nursery):
959+
"""test message send during local-initiated close handshake (issue #158)"""
960+
961+
async def handler(request: WebSocketRequest):
962+
await request.accept()
963+
await trio.sleep_forever()
964+
965+
server = await nursery.start(
966+
partial(serve_websocket, handler, HOST, 0, ssl_context=None))
967+
968+
client = await connect_websocket(nursery, HOST, server.port,
969+
RESOURCE, use_ssl=False)
970+
orig_send = client._send
971+
close_sent = trio.Event()
972+
973+
async def _send_wrapper(event):
974+
if isinstance(event, CloseConnection):
975+
close_sent.set()
976+
return await orig_send(event)
977+
978+
client._send = _send_wrapper
979+
assert not client.closed
980+
nursery.start_soon(client.aclose)
981+
await close_sent.wait()
982+
assert client.closed
983+
with pytest.raises(ConnectionClosed):
984+
await client.send_message('hello')
985+
986+
958987
@fail_after(DEFAULT_TEST_MAX_DURATION)
959988
async def test_server_tcp_closed_on_close_connection_event(nursery):
960989
"""ensure server closes TCP immediately after receiving CloseConnection"""

trio_websocket/_impl.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,7 @@ def __init__(self, code, reason):
493493
Constructor.
494494
495495
:param int code:
496-
:param str reason:
496+
:param Optional[str] reason:
497497
'''
498498
self._code = code
499499
try:
@@ -838,17 +838,22 @@ async def aclose(self, code=1000, reason=None): # pylint: disable=arguments-dif
838838
with _preserve_current_exception():
839839
await self._aclose(code, reason)
840840

841-
async def _aclose(self, code=1000, reason=None):
841+
async def _aclose(self, code, reason):
842842
if self._close_reason:
843843
# Per AsyncResource interface, calling aclose() on a closed resource
844844
# should succeed.
845845
return
846846
try:
847847
if self._wsproto.state == ConnectionState.OPEN:
848+
# Our side is initiating the close, so send a close connection
849+
# event to peer, while setting the local close reason to normal.
850+
self._close_reason = CloseReason(1000, None)
848851
await self._send(CloseConnection(code=code, reason=reason))
849852
elif self._wsproto.state in (ConnectionState.CONNECTING,
850853
ConnectionState.REJECTING):
851854
self._close_handshake.set()
855+
# TODO: shouldn't the receive channel be closed earlier, so that
856+
# get_message() during send of the CloseConneciton event fails?
852857
await self._recv_channel.aclose()
853858
await self._close_handshake.wait()
854859
except ConnectionClosed:

0 commit comments

Comments
 (0)