green.socket._wait_{read,write} consumes external gevent.Timeout's #242
Conversation
…the one started there
|
If you could add a test, this is definitely the right way to go. |
|
Such a test would be a simple confirmation that recv with nothing incoming is properly interrupted by a Timeout, and would belong in zmq.tests.test_socket:TestSocketGreen. Let me know if you are comfortable doing that, otherwise I can take it from here. |
|
I added a test and some cleanup when the wait gets interrupted (otherwise the socket gets stuck when interrupted), though I'm not sure if always resetting the AsyncResult breaks something somewhere else. I'm pretty new to unittesting, so any comments or corrections would be much appreciated. |
| g = gevent.spawn_later(0.2, lambda: a.send(b'hi')) | ||
| timeout = gevent.Timeout(0.1) | ||
| timeout.start() | ||
| with self.assertRaises(gevent.Timeout) as cm: |
There was a problem hiding this comment.
This syntax is not available in Python 2.6, so we need self.assertRaises(gevent.Timeout, b.recv)
Also, lets extend the timeout for g to a more comfortable distance (say, 0.5). We can get false failures when running tests on slow VMs if our times are too short.
This is the right thing to do. The way the code is written, only one greenlet can be waiting on a given event. If the AsyncResult is not set, that means that someone is currently waiting on the event (and concurrent attempts to wait on it should raise). Raising a timeout here means giving up on the wait, so it should be set. |
|
Perfect, thanks! |
Don't consume Timeouts in `gevent.socket._wait_{read,write}`
other timeouts could be consumed by the timeout used to workaround the gevent bug for missing events.
closes #241
see #241