ThreadGroup.stop() doesn't always stop threads

[Steven Hardy]

When attempting to stop running threads in a ThreadGroup via ThreadGroup.stop(), there is a problem because Thread.stop() uses greenthread.kill(), which doesn't really kill anything or prevent the greenthread being rescheduled, it just makes the greenthread raise a greenlet.GreenletExit exception.

This causes a problem if you happen to be in a try/except block which is not fully qualified (e.g it's either naked, or catches a generic Exception). In this instance, the thread does not exit, because the GreenletExit exception is caught, so you maybe see an error logged but then the thread will continue to run.

This can obviously cause very unexpected things to happen if you're expecting the threads in the ThreadGroup to have all been killed and they suddenly start running again... :(

Obviously the real problem here is eventlet, but raising this bug anyway so we can track the problem, and hopefully figure out a workaround.

The only solution I can see at the moment is to ensure every single try/except block in the code-path of the greenthread is fully-qualified, or that the except block contains logic to raise when a GreenletExit is detected. Obviously this is far from ideal, as there are a very large number of generic "except Exception" blocks in the various openstack projects.

I've attached a simple reproducer demonstrating the problem.

[Mark McLoughlin (markmc)]

Thanks Steven. Marking as Wishlist since you seem pretty confident there isn't an obvious fix

[Zane Bitter (zaneb)]

class GreenletReallyExit(BaseException):
    pass

greenlet.GreenletExit = GreenletReallyExit

sorted().

[Zane Bitter (zaneb)]

Scratch that; the GreenletExit defined in the eventlet package isn't the one actually raised. The error is buried in the python-greenlet package. Which, to make it even worse, is a C library so it can't even be monkey-patched :(

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/20021

[Mark McLoughlin (markmc)]

Seems to be fixed upstream in greenlet-0.3.2

https://github.com/python-greenlet/greenlet/commit/2f81f5d

It probably makes sense to require 0.3.2 to ensure we have this behaviour

Distros which are still shipping 0.3.1 (nearly 3 years old!) may choose to backport the fix though

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/20026

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/20026
Committed: http://github.com/openstack/oslo-incubator/commit/cc7acd7df86cf231e6673f48e58640adbc9e35a7
Submitter: Jenkins
Branch: master

commit cc7acd7df86cf231e6673f48e58640adbc9e35a7
Author: Zane Bitter <email address hidden>
Date: Fri Jan 18 14:52:33 2013 +0100

    Require greenlet 0.3.2 (or later)

    Version 0.3.2 resolves a bug that allowed generic "except Exception:"
    clauses to catch GreenletExit exceptions.

    bug 1097203

    Change-Id: Iecf36df36e5eefa45c70fae69c966bcd1f723c8f
    Signed-off-by: Zane Bitter <email address hidden>

[Alan Pevec (apevec)]

For anyone watching this space, greenlet fix was backported to Fedora18/17:
* Fri Jan 18 2013 Pádraig Brady <email address hidden> - 0.3.1-12
- Fix base exception type thrown

 http://pkgs.fedoraproject.org/cgit/python-greenlet.git/commit/?h=f18&id=a9feed2c525dda14317bd49f4da6d435aeeaf16c