Proper way to shut down greenlets

[Todd Sinclair]

Hi everyone,

We just started using gevent (1.0rc3 right now), and ran into an issue when trying to shut down cleanly.  We're making a modular server, where each module is pluggable, and is allowed to start greenlets on its own.  From the docs, the greenlet.kill() function claims to throw an exception into the specified greenlet, so that seemed perfect - just loop through each greenlet, throw a GreenletExit (or derived) exception.  Most of the developers writing modules wouldn't even need to do anything, they'd just let the exception fall through and exit normally.

However, it turns out kill() doesn't work as advertised.  It doesn't throw the exception immediately, it appears to add it to the greenlet's queue.  Easy to demonstrate:

def child_func():
  while True:
    print 'Child running'
    gevent.sleep()


child = gevent.spawn(child_func)
gevent.sleep()  # allow child to run
gevent.kill(child)
child.join()

The output shows "Child running" twice, and using greenlet.settrace() confirms the child runs through the loop one additional time after the kill call.  Changing gevent.sleep() in the child_func to gevent.sleep(1) results in the child exiting immediately after the kill call, as expected.  So it appears that gevent is not throwing the exception immediately, it's just placing the exception into the child's event queue, which always has the wakeup event from sleep in it.  At best, this delays shutdown as the child does extra work that is not necessary.

So what's the proper way to shut things down?  Is there a way to force the exception to be thrown immediately into each greenlet?  Tsing child.throw() makes the child exit immediately, but also causes the main thread to due claiming the throw() call would block forever.

Or is there a better way to accomplish this?

Thanks!

[Jonathan Kamens]

This is related:

https://github.com/surfly/gevent/issues/330

[Todd Sinclair]

Thanks, that explains the behavior.  The exception is added to the event loop, meaning greenlets won't be killed until all other messages currently pending from the loop get processed.

I suppose that makes sense a lot of the time, but our application is under constraints on how long it can take to shut down - take too long, and a watchdog process will forcibly core it.

As a test, I went in and added a run_callback_immediate function to core.ppyx, that inserts the new callback into the front of callback list, rather than appending to the end.  Modifying kill() to use that function instead of run_callback, and it kills the child right away without the extra loop iteration.

It does not appear that this would cause any problems.  Greenlets already have to deal with receiving events/callbacks when they're already dead, right?  So inserting the new event at the front should simply mean the other events get thrown out, which is what I want in this case.

Is there a better way?

[Jonathan Kamens]

On 11/20/2013 09:16 PM, Todd Sinclair wrote:

I suppose that makes sense a lot of the time, but our application is under constraints on how long it can take to shut down - take too long, and a watchdog process will forcibly core it.

Personally, I don't think it ever makes sense.

When you kill a process in Linux, it means kill NOW, not when you get around to it. Similarly when you kill a thread. When a greenlet is killed, it should die, not keep working for a while and then die.

As a test, I went in and added a run_callback_immediate function to core.ppyx, that inserts the new callback into the front of callback list, rather than appending to the end.  Modifying kill() to use that function instead of run_callback, and it kills the child right away without the extra loop iteration.

It does not appear that this would cause any problems.  Greenlets already have to deal with receiving events/callbacks when they're already dead, right?  So inserting the new event at the front should simply mean the other events get thrown out, which is what I want in this case.

Is there a better way?

I hope you will submit this to the maintainers of gevent as a PR, and I hope they'll merge it in, because I believe it is the correct behavior.

  jik