Closed belm0 closed 6 years ago
I'm not sure if that code is correct. It's definitely hacky and needs to be improved or replaced. Trio's upcoming channels feature sounds like it would work nicely, but it is unfinished.
Anyway, here's my reasoning behind that block. The goal is for any call to get_message()
on a closed connection to raise ConnectionClosed
, including calls made after the connection closed as well as calls made before the connection closed that are suspended in Queue.get()
.
Queue.put_nowait()
will raise WouldBlock
if no tasks are currently suspended in Queue.get()
.Queue.put_nowait()
and yield while another task is suspended in Queue.get()
, that task will wake up and consume the item from the queue.
Queue.get()
, then we need to wake up each task.Queue.put_nowait()
and it raises WouldBlock
, then we can reason that no tasks are suspended in get()
and we can exit this block.
get_message()
after we exit this block, it will immediately raise ConnectionClosed
, so there is no race condition where another queue consumer suddenly appears.Updated: I previously wrote that there was a bug in the ordering of the statements in lines 206-207, but on further reflection I think the ordering is fine.
Fixed in #30.
I had a chance to review the code briefly, looks very good. One thing standing out as a little odd was the
_close_message_queue()
implementation:https://github.com/HyperionGray/trio-websocket/blob/b787bf1a8a026ef1d9ca995d044bc97d42e7f727/trio_websocket/__init__.py#L204-L209
Possible issues:
put_nowait()
was successful on the first iteration, tasks waiting on the queue will certainly receive the exception object. So I can't see a reason for having the while loop which puts yet more exceptions in the queue.put_nowait()
fails on the first iteration, it means an unreceived message is already pending, and the while loop is exited. But in that case the tasks will not receive the close exception-- is that OK?