Commit b66e7121 authored by Jason Madden's avatar Jason Madden

Fix #1663 by more gracefully handling non-type arguments.

The documentation specifically says that kill, killall, etc, all take a a type, but
allow passing exception instances.

Also allow passing arbitrary objects; this is also not documented, and not recommended.
The result will be raising a BaseException in the greenlet. That should be better than
silently printing something to stderr, which is what happened before.
parent 166d5ce3
Incorrectly passing an exception *instance* instead of an exception
*type* to `gevent.Greenlet.kill` or `gevent.killall` no longer prints
an exception to stderr.
......@@ -449,7 +449,14 @@ class Greenlet(greenlet):
self._start_event.close()
def __handle_death_before_start(self, args):
# args is (t, v, tb) or simply t or v
# args is (t, v, tb) or simply t or v.
# The last two cases are transformed into (t, v, None);
# if the single argument is an exception type, a new instance
# is created; if the single argument is not an exception type and also
# not an exception, it is wrapped in a BaseException (this is not
# documented, but should result in better behaviour in the event of a
# user error---instead of silently printing something to stderr, we still
# kill the greenlet).
if self._exc_info is None and self.dead:
# the greenlet was never switched to before and it will
# never be; _report_error was not called, the result was
......@@ -462,12 +469,15 @@ class Greenlet(greenlet):
# the greenlet).
if len(args) == 1:
arg = args[0]
if issubclass(arg, BaseException):
if isinstance(arg, type) and issubclass(arg, BaseException):
args = (arg, arg(), None)
else:
args = (type(arg), arg, None)
elif not args:
args = (GreenletExit, GreenletExit(), None)
if not issubclass(args[0], BaseException):
# Random non-type, non-exception arguments.
args = (BaseException, BaseException(args), None)
assert issubclass(args[0], BaseException)
self.__report_error(args)
......@@ -734,7 +744,7 @@ class Greenlet(greenlet):
a :meth:`link` or :meth:`rawlink` (cheaper) may be a safer way to
clean up resources.
See also :func:`gevent.kill`.
See also :func:`gevent.kill` and :func:`gevent.killall`.
:keyword type exception: The type of exception to raise in the greenlet. The default
is :class:`GreenletExit`, which indicates a :meth:`successful` completion
......@@ -1096,7 +1106,7 @@ def killall(greenlets, exception=GreenletExit, block=True, timeout=None):
calling this method will never be switched to. This makes this function
behave like :meth:`Greenlet.kill`. This does not apply to raw greenlets.
.. versionchanged:: 1.5a3
Now accepts raw greenlets created by :func:`gevent.spawn_raw`.
Now accepts raw greenlets created by :func:`gevent.spawn_raw`.
"""
need_killed = [] # type: list
......
......@@ -732,33 +732,43 @@ class TestBasic(greentest.TestCase):
class TestKill(greentest.TestCase):
def __assertKilled(self, g):
def __assertKilled(self, g, successful):
self.assertFalse(g)
self.assertTrue(g.dead)
self.assertFalse(g.started)
self.assertTrue(g.ready())
self.assertTrue(g.successful(), (repr(g), g.value, g.exception))
self.assertIsInstance(g.value, gevent.GreenletExit)
self.assertIsNone(g.exception)
if successful:
self.assertTrue(g.successful(), (repr(g), g.value, g.exception))
self.assertIsInstance(g.value, gevent.GreenletExit)
self.assertIsNone(g.exception)
else:
self.assertFalse(g.successful(), (repr(g), g.value, g.exception))
self.assertNotIsInstance(g.value, gevent.GreenletExit)
self.assertIsNotNone(g.exception)
def assertKilled(self, g):
self.__assertKilled(g)
def assertKilled(self, g, successful=True):
self.__assertKilled(g, successful)
gevent.sleep(0.01) # spin the loop to make sure it doesn't run.
self.__assertKilled(g)
self.__assertKilled(g, successful)
def __kill_greenlet(self, g, block, killall):
def __kill_greenlet(self, g, block, killall, exc=None):
if exc is None:
exc = gevent.GreenletExit
if killall:
killer = functools.partial(gevent.killall, [g], block=block)
killer = functools.partial(gevent.killall, [g],
exception=exc, block=block)
else:
killer = functools.partial(g.kill, block=block)
killer = functools.partial(g.kill, exception=exc, block=block)
killer()
if not block:
# Must spin the loop to take effect (if it was scheduled)
gevent.sleep(timing.SMALLEST_RELIABLE_DELAY)
self.assertKilled(g)
successful = exc is None or (isinstance(exc, type) and issubclass(exc, gevent.GreenletExit))
self.assertKilled(g, successful)
# kill second time must not hurt
killer()
self.assertKilled(g)
self.assertKilled(g, successful)
@staticmethod
def _run_in_greenlet(result_collector):
......@@ -772,7 +782,7 @@ class TestKill(greentest.TestCase):
_after_kill_greenlet = _start_greenlet
def _do_test(self, block, killall):
def _do_test(self, block, killall, exc=None):
link_test = []
result = []
g = gevent.Greenlet(self._run_in_greenlet, result)
......@@ -780,7 +790,7 @@ class TestKill(greentest.TestCase):
self._start_greenlet(g)
self.__kill_greenlet(g, block, killall)
self.__kill_greenlet(g, block, killall, exc)
self._after_kill_greenlet(g)
......@@ -793,12 +803,36 @@ class TestKill(greentest.TestCase):
def test_non_block(self):
self._do_test(block=False, killall=False)
def test_block_killal(self):
def test_block_killall(self):
self._do_test(block=True, killall=True)
def test_non_block_killal(self):
self._do_test(block=False, killall=True)
def test_non_type_exception(self):
self._do_test(block=True, killall=False, exc=Exception())
def test_non_type_exception_non_block(self):
self._do_test(block=False, killall=False, exc=Exception())
def test_non_type_exception_killall(self):
self._do_test(block=True, killall=True, exc=Exception())
def test_non_type_exception_killall_non_block(self):
self._do_test(block=False, killall=True, exc=Exception())
def test_non_exc_exception(self):
self._do_test(block=True, killall=False, exc=42)
def test_non_exc_exception_non_block(self):
self._do_test(block=False, killall=False, exc=42)
def test_non_exc_exception_killall(self):
self._do_test(block=True, killall=True, exc=42)
def test_non_exc_exception_killall_non_block(self):
self._do_test(block=False, killall=True, exc=42)
class TestKillAfterStart(TestKill):
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment