traviscline/gevent-zeromq — Issue #22: context.term() deadlock
Author: zacharyvoase
Created: 2012-01-05 23:23:53+00:00
State: open

This is an issue I ran into when using `context.term()` to prompt application
shutdown whilst multiple greenlets were actively using sockets.

What *should* happen (according to [the docs][1]) when I call `context.term()`
is that any ongoing socket operations are interrupted by an `ETERM`, giving
their controlling threads the opportunity to close the socket. The `term()` call
blocks until all the context's sockets have been `close()`-d. However, due to
this bug, the controlling greenlets were never being notified of the `ETERM`, so
attempting to terminate the context sent the entire application into deadlock.

[1]: http://api.zeromq.org/2-1:zmq-term

## Demonstration of the bug

```python
import threading
import gevent
from gevent_zeromq import zmq

ctx = zmq.Context()
pusher = ctx.socket(zmq.PUSH)
pusher.bind('inproc://pusher')

def puller():
sock = ctx.socket(zmq.PULL)
sock.connect('inproc://pusher')
try:
sock.recv()
except zmq.ZMQError, exc:
if exc.errno == zmq.ETERM:
return
raise
finally:
sock.close()

try:
puller1 = gevent.spawn(puller)
puller2 = gevent.spawn(puller)
gevent.joinall([puller1, puller2])
except KeyboardInterrupt:
pass
finally:
# `ctx.term()` is started in a thread because it's a blocking (although
# GIL-releasing) system call. I'll work on a non-blocking version soon.
t = threading.Thread(target=ctx.term)
t.start()
pusher.close()
gevent.sleep(0)
t.join() # Hangs here, never exits and can't be Ctrl-C'd.
```

Note that if I use threads all the way, and the default zmq library instead of
gevent-zeromq, this issue goes away.

## First Problem & Solution

There are two parts to the problem. The first is how socket state changes are
handled (in the [`core.Socket.__state_changed()` method][2]). There was a naked
call to `self.getsockopt(EVENTS)`, and if the context had been terminated it was
*this* call that raised the `ETERM`. When this happened, control flow had not
yet passed to the waiting greenlet, so the greenlet was left blocking on an
event which would never be `set()`.

[2]: https://github.com/traviscline/gevent-zeromq/blob/24eb2639/gevent_zeromq/core.pyx#L79

I fixed this in dvxhouse/gevent-zeromq@a0ca3e7 by switching the read/write
events from `gevent.event.Event` instances to [`gevent.event.AsyncResult`][3].
Whereas `Event` supports only `set()` and `wait()`, `AsyncResult` also supports
throwing an exception in the waiting thread (via `set()`, `set_exception()` and
`get()` methods). Then, I wrapped the call to `getsockopt()` in a try/except,
capturing any `ZMQError`s and sending those to the read and write events.

[3]: http://www.gevent.org/gevent.event.html#gevent.event.AsyncResult

I could have just set the events and allowed the `ETERM` error to be raised on
the `recv()`/`send()` calls, but I feel this approach is more generic—we might
get other exceptions from the `getsockopt()` call and now they will all be
forwarded through to the waiting greenlet.

## Second Problem & Solution

The second problem then occurs in the calls to `socket.close()`. Another `ETERM`
is raised here, which is surprising and annoying because this method *needs* to
succeed in order for the context termination to complete. It turns out that
gevent-zeromq's overridden `Socket.close()` actually inspects `self.closed`,
which is a computed property that results in a call to `zmq_getsockopt()` again.
See the following functions (going down the stack):

* gevent_zeromq: `Socket.close()`: https://github.com/traviscline/gevent-zeromq/blob/24eb263/gevent_zeromq/core.pyx#L60
* pyzmq: `zmq.Socket.closed`: https://github.com/zeromq/pyzmq/blob/c2bef8d/zmq/core/socket.pyx#L226
* pyzmq: `zmq.core.socket._check_closed()`: https://github.com/zeromq/pyzmq/blob/c2bef8d/zmq/core/socket.pyx#L77

The solution here is instead to use the `_closed` attribute rather than the
computed property, as I've done in dvxhouse/gevent-zeromq@a1c2d1dc.

With both these fixes, it seems the problem is finally dead. You can try running
that demo code above; give it a couple of seconds and hit Ctrl-C; it'll exit
right away. I'll submit a corresponding pull request now, too.