Fix blocking issue in Voila (nbclient case) #1059
Conversation
|
|
|
@davidbrochart @JohanMabille @minrk friendly ping, would you have any thought on this? |
| @@ -810,10 +810,6 @@ def send( | |||
| # ZMQStreams and dummy sockets do not support tracking. | |||
| track = False | |||
|
|
|||
| if isinstance(stream, zmq.asyncio.Socket): | |||
| assert stream is not None # type:ignore[unreachable] | |||
| stream = zmq.Socket.shadow(stream.underlying) | |||
There was a problem hiding this comment.
without this, it's not actually guaranteed the send will happen, since it's not awaited (zmq.asyncio does currently immediately schedule sends, but it doesn't guarantee that it will continue to do so), and the type of tracker will be incorrect Awaitable[Event] instead of Event (and a different type when copy=True vs copy=False.
We could add async polling, either here or (perhaps easier) in nbclient, so you can avoid calling this when it would block:
events = await async_socket.poll(timeout_ms, zmq.POLLOUT)
if events & zmq.POLLOUT:
session.send(async_socket) # won't block
else:
# ????
There was a problem hiding this comment.
I might be missing some bits here, but would it be possible to handle that in the AsyncZMQSocketChannel instead? I.e. overwrite the send method, do the proper stuff, and then call session.send? This would prevent the Socket wrapper from leaking into the Session class.
There was a problem hiding this comment.
Yeah, that's probably where it belongs
Fixing voila-dashboards/voila#1428
The issue is that when using the
AsyncKernelMappingManagerin Voila, we asynchronously poll messages, but on the jupyter-client side we have a synchronous socket. Asynchronously waiting for a message on a synchronous socket seems to be the culprit for Voila getting stuck.