WebSocketConnection should implement the trio.abc.AsyncResource interface

[njsmith]

This means:

• Rename the close method to aclose
• Inherit from AsyncResource, so isinstance checks work and you get __aenter__ and __aexit__ methods for free
• Make sure that if aclose is cancelled, then it closes the underlying connection and cleans everything up, without blocking

(Docs here)

The last one is a bit tricky... you need to think about the case where both the call to aclose and the writer task are cancelled together, or the case where just the aclose call is cancelled, and in both cases shut down the reader and writer tasks as well as closing the underlying socket.

It might be simpler to get rid of the writer task entirely? Instead, you could call send_all from inside the task that's trying to send something. Like, here's a simplified sketch of what I'm imagining:

def _flush_sends(self):
    async with self._send_lock:
        data = self._wsproto.bytes_to_send()
        if data:
            await self._stream.send_all(data)

async def send_message(self, message):
    self._wsproto.send_data(message)
    await self._flush_sends()

async def aclose(self, code=1000, reason=None):
    try:
        self._wsproto.close(code=code, reason=reason)
        await self._flush_sends()
    finally:
        # We do this even if _flush_sends was cancelled
        await self._stream.aclose()

I guess this will also kill the reader indirectly, by causing its current-or-next call to receive_some to raise ClosedResourceError. We might want to catch that and handle it somehow.

It would also be good if closing the websocket caused any waiting calls to get_message to raise an error. That will be easier once python-trio/trio#586 lands and we can close the message queue.

[mehaase]

Working on this right now. The first two points are clear to me, but I'm a bit confused by the third:

Make sure that if aclose is cancelled, then it closes the underlying connection and cleans everything up, without blocking

Do you mean "blocking" in the synchronous sense, i.e. the entire event loop is blocked, or do you mean "blocking" as in never yielding? If it's the latter, then isn't await self._stream.aclose() a yield point?

[njsmith]

Do you mean "blocking" in the synchronous sense, i.e. the entire event loop is blocked, or do you mean "blocking" as in never yielding?

Neither :-). I mean, performing operations that take an unknown, possibly large, amount of time. In particular, a regular aclose might send a CLOSE message, but a cancelled aclose shouldn't.

This is probably not too hard to accomplish, since if the aclose is cancelled then attempting to send data will raise a Cancelled exception. So you can probably write the code to just do what it would normally do, so long as you make sure to put the call to aclose inside a finally block so that it execute even if the normal shutdown code raises Cancelled.

[mehaase]

Fixed in #24.