Add working pyzmq socket support #3
Conversation
I'd ignored what would happen if there were multiple send tasks waiting
There was a problem hiding this comment.
One substantive question, but overall yeah this all looks reasonable!
Obviously to make a "real" package we also need tests and docs, and probably to wrap a bit more of the zmq API (e.g. Socket.setsockopt).
Are you planning to keep going, or was this more of just an exercise to see if you could make it work? If the latter that's cool, but then we should probably update the README a bit to explain the new status before leaving it for the next person :-).
| await trio.lowlevel.wait_readable(self._zmq_sock.fd) | ||
| finally: | ||
| self._someone_is_watching_trigger = False | ||
| self._update_events() |
There was a problem hiding this comment.
The old code had wake_all=True here and a comment explaining why -- is that still needed or are you handling that somewhere else somehow?
There was a problem hiding this comment.
I had convinced myself it wasn't needed, because the rule is that anyone who wakes up will do whatever they wanted to do and then call _update_events.
I was just writing an example to show that was right, and instead I think I've come up with an example that DOES need wake_all, so I guess I'm wrong.
If all the waiters are wanting to write: one will end up watching the trigger and one will end up sleeping. If the watcher wakes up, it will do its write, and then call _update_events which doesn't wake up the other waiter, and now no one is listening for the trigger.
In short, I think you're right and I should add it back.
There was a problem hiding this comment.
No, that's not correct.
In that example the other waiter would be awoken (the event is broadcast to all of the same type of waiters) and end up as the next watcher when it discovers the socket isn't writable anymore.
So that means the case of "watcher and waiter are the same type" is ok.
For the case of "watcher and waiter are of different types". The watcher will wake up: if the event matches the waiter, it's obviously ok because the watcher will go back to watching. If the event matches the watcher, the watcher will leave _wait_for and temporarily no one is watching the trigger. The watcher then does whatever it wanted to do and then calls _update_events.
My assertion is that this will always cause the waiter to be awoken (if the watcher wrote, then the waiter must have been waiting for a message, and it will now see that there is one to read. If the watcher read, then the waiter must have been waiting for space to write a message and it will now be available.)
There was a problem hiding this comment.
Is that taking into account that the reason the watcher might wake up is b/c they were cancelled?
Just pushing the cart a little further. This at least seems to be basically functional.
Am I missing anything you can see immediately?