Make gevent.killall do what Greenlet.kill does.

And stop a fresh greenlet from even running.

Fixes #1473.

Free user-provided resources as soon as the greenlet is cancelled, if it's not running.

.pylintrc

@@ -101,10 +101,11 @@ dummy-variables-rgx=_.*
 generated-members=exc_clear
 
 # List of classes names for which member attributes should not be checked
-# (useful for classes with attributes dynamically set). This supports can work
+# (useful for classes with attributes dynamically set). This can work
 # with qualified names.
 #ignored-classes=SSLContext, SSLSocket, greenlet, Greenlet, parent, dead
 
+
 # List of module names for which member attributes should not be checked
 # (useful for modules/projects where namespaces are manipulated during runtime
 # and thus existing member attributes cannot be deduced by static analysis. It

CHANGES.rst

@@ -81,6 +81,19 @@
   raised an exception. This could happen if the hub's ``handle_error``
   function was poorly customized, for example. See :issue:`1482`
 
+- Make ``gevent.killall`` stop greenlets from running that hadn't been
+  run yet. This make it consistent with ``Greenlet.kill()``. See
+  :issue:`1473` reported by kochelmonster.
+
+- Make ``gevent.spawn_raw`` set the ``loop`` attribute on returned
+  greenlets. This lets them work with more gevent APIs, notably
+  ``gevent.killall()``. They already had dictionaries, but this may
+  make them slightly larger, depending on platform (on CPython 2.7
+  through 3.6 there is no apparent difference for one attribute but on
+  CPython 3.7 and 3.8 dictionaries are initially empty and only
+  allocate space once an attribute is added; they're still smaller
+  than on earlier versions though).
+
 1.5a2 (2019-10-21)
 ==================
 

pyproject.toml

@@ -12,9 +12,9 @@ requires = [
      # 0.28 is faster, and (important!) lets us specify the target module
      # name to be created so that we can have both foo.py and _foo.so
      # at the same time. 0.29 fixes some issues with Python 3.7,
-     # and adds the 3str mode for transition to Python 3. 0.29.12+ is
+     # and adds the 3str mode for transition to Python 3. 0.29.14+ is
      # required for Python 3.8
-     "Cython >= 0.29.13",
+     "Cython >= 0.29.14",
      # See version requirements in setup.py
      "cffi >= 1.12.3 ; platform_python_implementation == 'CPython'",
      # Python 3.7 requires at least 0.4.14, which is ABI incompatible with earlier

src/gevent/_ffi/watcher.py

@@ -10,6 +10,7 @@ import functools
 import warnings
 
 from gevent._config import config
+from gevent._util import LazyOnClass
 
 try:
     from tracemalloc import get_object_traceback
@@ -108,26 +109,6 @@ def only_if_watcher(func):
     return if_w
 
 
-class LazyOnClass(object):
-
-    @classmethod
-    def lazy(cls, cls_dict, func):
-        "Put a LazyOnClass object in *cls_dict* with the same name as *func*"
-        cls_dict[func.__name__] = cls(func)
-
-    def __init__(self, func, name=None):
-        self.name = name or func.__name__
-        self.func = func
-
-    def __get__(self, inst, klass):
-        if inst is None: # pragma: no cover
-            return self
-
-        val = self.func(inst)
-        setattr(klass, self.name, val)
-        return val
-
-
 class AbstractWatcherType(type):
     """
     Base metaclass for watchers.

src/gevent/_greenlet.pxd

@@ -133,6 +133,9 @@ cdef class Greenlet(greenlet):
     cpdef rawlink(self, object callback)
     cpdef str _formatinfo(self)
 
+    # Helper for killall()
+    cpdef bint _maybe_kill_before_start(self, exception)
+
     # This is a helper function for a @property getter;
     # defining locals() for a @property doesn't seem to work.
     @cython.locals(reg=IdentRegistry)
@@ -144,11 +147,13 @@ cdef class Greenlet(greenlet):
     cdef bint __never_started_or_killed(self)
     cdef bint __start_completed(self)
     cdef __handle_death_before_start(self, tuple args)
+    @cython.final
+    cdef inline void __free(self)
 
     cdef __cancel_start(self)
 
-    cdef _report_result(self, object result)
-    cdef _report_error(self, tuple exc_info)
+    cdef inline __report_result(self, object result)
+    cdef inline __report_error(self, tuple exc_info)
     # This is used as the target of a callback
     # from the loop, and so needs to be a cpdef
     cpdef _notify_links(self)

src/gevent/_util.py

@@ -119,7 +119,9 @@ class Lazy(object):
     A non-data descriptor used just like @property. The
     difference is the function value is assigned to the instance
     dict the first time it is accessed and then the function is never
-    called agoin.
+    called again.
+
+    Contrast with `readproperty`.
     """
     def __init__(self, func):
         self.data = (func, func.__name__)
@@ -136,9 +138,16 @@ class Lazy(object):
 
 class readproperty(object):
     """
-    A non-data descriptor like @property. The difference is that
-    when the property is assigned to, it is cached in the instance
-    and the function is not called on that instance again.
+    A non-data descriptor similar to :class:`property`.
+
+    The difference is that the property can be assigned to directly,
+    without invoking a setter function. When the property is assigned
+    to, it is cached in the instance and the function is not called on
+    that instance again.
+
+    Contrast with `Lazy`, which caches the result of the function in the
+    instance the first time it is called and never calls the function on that
+    instance again.
     """
 
     def __init__(self, func):
@@ -151,6 +160,35 @@ class readproperty(object):
 
         return self.func(inst)
 
+class LazyOnClass(object):
+    """
+    Similar to `Lazy`, but stores the value in the class.
+
+    This is useful when the getter is expensive and conceptually
+    a shared class value, but we don't want import-time side-effects
+    such as expensive imports because it may not always be used.
+
+    Probably doesn't mix well with inheritance?
+    """
+
+    @classmethod
+    def lazy(cls, cls_dict, func):
+        "Put a LazyOnClass object in *cls_dict* with the same name as *func*"
+        cls_dict[func.__name__] = cls(func)
+
+    def __init__(self, func, name=None):
+        self.name = name or func.__name__
+        self.func = func
+
+    def __get__(self, inst, klass):
+        if inst is None: # pragma: no cover
+            return self
+
+        val = self.func(inst)
+        setattr(klass, self.name, val)
+        return val
+
+
 def gmctime():
     """
     Returns the current time as a string in RFC3339 format.

src/gevent/greenlet.py

 # Copyright (c) 2009-2012 Denis Bilenko. See LICENSE for details.
 # cython: auto_pickle=False,embedsignature=True,always_allow_keywords=False
-
+# pylint:disable=too-many-lines
 from __future__ import absolute_import, print_function, division
 
 from sys import _getframe as sys_getframe
@@ -260,8 +260,10 @@ class Greenlet(greenlet):
         #: start() and start_later() as those two objects, respectively.
         #: Once this becomes non-None, the Greenlet cannot be started again. Conversely,
         #: kill() and throw() check for non-None to determine if this object has ever been
-        #: scheduled for starting. A placeholder _dummy_event is assigned by them to prevent
+        #: scheduled for starting. A placeholder _cancelled_start_event is assigned by them to prevent
         #: the greenlet from being started in the future, if necessary.
+        #: In the usual case, this transitions as follows: None -> event -> _start_completed_event.
+        #: A value of None means we've never been started.
         self._start_event = None
 
         self._notifier = None
@@ -393,15 +395,31 @@ class Greenlet(greenlet):
     else:
         @property
         def dead(self):
-            "Boolean indicating that the greenlet is dead and will not run again."
-            return self.__start_cancelled_by_kill() or self.__started_but_aborted() or greenlet.dead.__get__(self)
+            """
+            Boolean indicating that the greenlet is dead and will not run again.
+
+            This is true if:
+
+            1. We were never started, but were :meth:`killed <kill>`
+               immediately after creation (not possible with :meth:`spawn`); OR
+            2. We were started, but were killed before running; OR
+            3. We have run and terminated (by raising an exception out of the
+               started function or by reaching the end of the started function).
+            """
+            return (
+                self.__start_cancelled_by_kill()
+                or self.__started_but_aborted()
+                or greenlet.dead.__get__(self)
+            )
 
     def __never_started_or_killed(self):
         return self._start_event is None
 
     def __start_pending(self):
-        return (self._start_event is not None
-                and (self._start_event.pending or getattr(self._start_event, 'active', False)))
+        return (
+            self._start_event is not None
+            and (self._start_event.pending or getattr(self._start_event, 'active', False))
+        )
 
     def __start_cancelled_by_kill(self):
         return self._start_event is _cancelled_start_event
@@ -410,10 +428,12 @@ class Greenlet(greenlet):
         return self._start_event is _start_completed_event
 
     def __started_but_aborted(self):
-        return (not self.__never_started_or_killed() # we have been started or killed
-                and not self.__start_cancelled_by_kill() # we weren't killed, so we must have been started
-                and not self.__start_completed() # the start never completed
-                and not self.__start_pending()) # and we're not pending, so we must have been aborted
+        return (
+            not self.__never_started_or_killed() # we have been started or killed
+            and not self.__start_cancelled_by_kill() # we weren't killed, so we must have been started
+            and not self.__start_completed() # the start never completed
+            and not self.__start_pending() # and we're not pending, so we must have been aborted
+        )
 
     def __cancel_start(self):
         if self._start_event is None:
@@ -431,20 +451,25 @@ class Greenlet(greenlet):
     def __handle_death_before_start(self, args):
         # args is (t, v, tb) or simply t or v
         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 not set and the links weren't notified. let's do it here.
-            # checking that self.dead is true is essential, because throw() does not necessarily kill the greenlet
-            # (if the exception raised by throw() is caught somewhere inside the greenlet).
+            # the greenlet was never switched to before and it will
+            # never be; _report_error was not called, the result was
+            # not set, and the links weren't notified. Let's do it
+            # here.
+            #
+            # checking that self.dead is true is essential, because
+            # throw() does not necessarily kill the greenlet (if the
+            # exception raised by throw() is caught somewhere inside
+            # the greenlet).
             if len(args) == 1:
                 arg = args[0]
-                #if isinstance(arg, type):
-                if type(arg) is type(Exception):
+                if issubclass(arg, BaseException):
                     args = (arg, arg(), None)
                 else:
                     args = (type(arg), arg, None)
             elif not args:
                 args = (GreenletExit, GreenletExit(), None)
-            self._report_error(args)
+            assert issubclass(args[0], BaseException)
+            self.__report_error(args)
 
     @property
     def started(self):
@@ -661,14 +686,27 @@ class Greenlet(greenlet):
         g.start_later(seconds)
         return g
 
+    def _maybe_kill_before_start(self, exception):
+        # Helper for Greenlet.kill(), and also for killall()
+        self.__cancel_start()
+        self.__free()
+        dead = self.dead
+        if dead:
+            self.__handle_death_before_start((exception,))
+        return dead
+
     def kill(self, exception=GreenletExit, block=True, timeout=None):
         """
         Raise the ``exception`` in the greenlet.
 
-        If ``block`` is ``True`` (the default), wait until the greenlet dies or the optional timeout expires.
+        If ``block`` is ``True`` (the default), wait until the greenlet
+        dies or the optional timeout expires; this may require switching
+        greenlets.
         If block is ``False``, the current greenlet is not unscheduled.
 
-        The function always returns ``None`` and never raises an error.
+        This function always returns ``None`` and never raises an error. It
+        may be called multpile times on the same greenlet object, and may be
+        called on an unstarted or dead greenlet.
 
         .. note::
 
@@ -687,7 +725,9 @@ class Greenlet(greenlet):
 
             Use care when killing greenlets. If the code executing is not
             exception safe (e.g., makes proper use of ``finally``) then an
-            unexpected exception could result in corrupted state.
+            unexpected exception could result in corrupted state. Using
+            a :meth:`link` or :meth:`rawlink` (cheaper) may be a safer way to
+            clean up resources.
 
         See also :func:`gevent.kill`.
 
@@ -698,21 +738,17 @@ class Greenlet(greenlet):
         .. versionchanged:: 0.13.0
             *block* is now ``True`` by default.
         .. versionchanged:: 1.1a2
-            If this greenlet had never been switched to, killing it will prevent it from ever being switched to.
+            If this greenlet had never been switched to, killing it will
+            prevent it from *ever* being switched to. Links (:meth:`rawlink`)
+            will still be executed, though.
         """
-        self.__cancel_start()
-
-        if self.dead:
-            self.__handle_death_before_start((exception,))
-        else:
+        if not self._maybe_kill_before_start(exception):
             waiter = Waiter() if block else None # pylint:disable=undefined-variable
             hub = get_my_hub(self) # pylint:disable=undefined-variable
             hub.loop.run_callback(_kill, self, exception, waiter)
             if waiter is not None:
                 waiter.get()
                 self.join(timeout)
-        # it should be OK to use kill() in finally or kill a greenlet from more than one place;
-        # thus it should not raise when the greenlet is already killed (= not started)
 
     def get(self, block=True, timeout=None):
         """
@@ -786,16 +822,16 @@ class Greenlet(greenlet):
             self.unlink(switch)
             raise
 
-    def _report_result(self, result):
+    def __report_result(self, result):
         self._exc_info = (None, None, None)
         self.value = result
         if self._links and not self._notifier:
             hub = get_my_hub(self) # pylint:disable=undefined-variable
             self._notifier = hub.loop.run_callback(self._notify_links)
 
-    def _report_error(self, exc_info):
+    def __report_error(self, exc_info):
         if isinstance(exc_info[1], GreenletExit):
-            self._report_result(exc_info[1])
+            self.__report_result(exc_info[1])
             return
 
         self._exc_info = exc_info[0], exc_info[1], dump_traceback(exc_info[2])
@@ -817,13 +853,23 @@ class Greenlet(greenlet):
             try:
                 result = self._run(*self.args, **self.kwargs)
             except: # pylint:disable=bare-except
-                self._report_error(sys_exc_info())
-                return
-            self._report_result(result)
+                self.__report_error(sys_exc_info())
+            else:
+                self.__report_result(result)
         finally:
-            self.__dict__.pop('_run', None)
-            self.args = ()
-            self.kwargs.clear()
+            self.__free()
+
+    def __free(self):
+        try:
+            # It seems that Cython 0.29.13 sometimes miscompiles
+            # self.__dict__.pop('_run', None) ? When we moved this out of the
+            # inline finally: block in run(), we started getting strange
+            # exceptions from places that subclassed Greenlet.
+            del self._run
+        except AttributeError:
+            pass
+        self.args = ()
+        self.kwargs.clear()
 
     def _run(self):
         """
@@ -850,7 +896,9 @@ class Greenlet(greenlet):
         The *callback* will be called with this instance as an
         argument.
 
-        .. caution:: The callable will be called in the HUB greenlet.
+        .. caution::
+            The *callback* will be called in the hub and
+            **MUST NOT** raise an exception.
         """
         if not callable(callback):
             raise TypeError('Expected callable: %r' % (callback, ))
@@ -1007,15 +1055,16 @@ _spawn_callbacks = None
 
 def killall(greenlets, exception=GreenletExit, block=True, timeout=None):
     """
-    Forceably terminate all the ``greenlets`` by causing them to raise ``exception``.
+    Forceably terminate all the *greenlets* by causing them to raise *exception*.
 
     .. caution:: Use care when killing greenlets. If they are not prepared for exceptions,
        this could result in corrupted state.
 
     :param greenlets: A **bounded** iterable of the non-None greenlets to terminate.
        *All* the items in this iterable must be greenlets that belong to the same hub,
-       which should be the hub for this current thread.
-    :keyword exception: The exception to raise in the greenlets. By default this is
+       which should be the hub for this current thread. If this is a generator or iterator
+       that switches greenlets, the results are undefined.
+    :keyword exception: The type of exception to raise in the greenlets. By default this is
         :class:`GreenletExit`.
     :keyword bool block: If True (the default) then this function only returns when all the
         greenlets are dead; the current greenlet is unscheduled during that process.
@@ -1031,15 +1080,38 @@ def killall(greenlets, exception=GreenletExit, block=True, timeout=None):
     .. versionchanged:: 1.1a2
         *greenlets* can be any iterable of greenlets, like an iterator or a set.
         Previously it had to be a list or tuple.
+    .. versionchanged:: 1.5a3
+        Any :class:`Greenlet` in the *greenlets* list that hadn't been switched to before
+        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`.
     """
-    # support non-indexable containers like iterators or set objects
-    greenlets = list(greenlets)
-    if not greenlets:
+
+    need_killed = [] # type: list
+    for glet in greenlets:
+        # Quick pass through to prevent any greenlet from
+        # actually being switched to if it hasn't already.
+        # (Previously we called ``list(greenlets)`` so we're still
+        # linear.)
+        #
+        # We don't use glet.kill() here because we don't want to schedule
+        # any callbacks in the loop; we're about to handle that more directly.
+        try:
+            cancel = glet._maybe_kill_before_start
+        except AttributeError:
+            need_killed.append(glet)
+        else:
+            if not cancel(exception):
+                need_killed.append(glet)
+
+    if not need_killed:
         return
-    loop = greenlets[0].loop
+
+    loop = glet.loop # pylint:disable=undefined-loop-variable
     if block:
         waiter = Waiter() # pylint:disable=undefined-variable
-        loop.run_callback(_killall3, greenlets, exception, waiter)
+        loop.run_callback(_killall3, need_killed, exception, waiter)
         t = Timeout._start_new_or_dummy(timeout)
         try:
             alive = waiter.get()
@@ -1048,7 +1120,7 @@ def killall(greenlets, exception=GreenletExit, block=True, timeout=None):
         finally:
             t.cancel()
     else:
-        loop.run_callback(_killall, greenlets, exception)
+        loop.run_callback(_killall, need_killed, exception)
 
 def _init():
     greenlet_init() # pylint:disable=undefined-variable

src/gevent/hub.py

@@ -96,12 +96,16 @@ def spawn_raw(function, *args, **kwargs):
        if ``GEVENT_TRACK_GREENLET_TREE`` is enabled (the default). If not enabled,
        those attributes will not be set.
 
+    .. versionchanged:: 1.5a3
+       The returned greenlet always has a *loop* attribute matching the
+       current hub's loop. This helps it work better with more gevent APIs.
     """
     if not callable(function):
         raise TypeError("function must be callable")
 
     # The hub is always the parent.
     hub = _get_hub_noargs()
+    loop = hub.loop
 
     factory = TrackedRawGreenlet if GEVENT_CONFIG.track_greenlet_tree else RawGreenlet
 
@@ -111,11 +115,11 @@ def spawn_raw(function, *args, **kwargs):
     if kwargs:
         function = _functools_partial(function, *args, **kwargs)
         g = factory(function, hub)
-        hub.loop.run_callback(g.switch)
+        loop.run_callback(g.switch)
     else:
         g = factory(function, hub)
-        hub.loop.run_callback(g.switch, *args)
-
+        loop.run_callback(g.switch, *args)
+    g.loop = hub.loop
     return g
 
 

src/gevent/testing/testcase.py

@@ -27,6 +27,7 @@ from unittest import TestCase as BaseTestCase
 from functools import wraps
 
 import gevent
+from gevent._util import LazyOnClass
 
 from . import sysinfo
 from . import params
@@ -77,6 +78,66 @@ class TimeAssertMixin(object):
             fuzzy=(0.01 if not sysinfo.EXPECT_POOR_TIMER_RESOLUTION and not sysinfo.LIBUV else 1.0)):
         return self.runs_in_given_time(0.0, fuzzy)
 
+
+class GreenletAssertMixin(object):
+    """Assertions related to greenlets."""
+
+    def assert_greenlet_ready(self, g):
+        self.assertTrue(g.dead, g)
+        self.assertTrue(g.ready(), g)
+        self.assertFalse(g, g)
+
+    def assert_greenlet_not_ready(self, g):
+        self.assertFalse(g.dead, g)
+        self.assertFalse(g.ready(), g)
+
+    def assert_greenlet_spawned(self, g):
+        self.assertTrue(g.started, g)
+        self.assertFalse(g.dead, g)
+
+    # No difference between spawned and switched-to once
+    assert_greenlet_started = assert_greenlet_spawned
+
+    def assert_greenlet_finished(self, g):
+        self.assertFalse(g.started, g)
+        self.assertTrue(g.dead, g)
+
+
+class StringAssertMixin(object):
+    """
+    Assertions dealing with strings.
+    """
+
+    @LazyOnClass
+    def HEX_NUM_RE(self):
+        import re
+        return re.compile('-?0x[0123456789abcdef]+L?', re.I)
+
+    def normalize_addr(self, s, replace='X'):
+        # https://github.com/PyCQA/pylint/issues/1127
+        return self.HEX_NUM_RE.sub(replace, s) # pylint:disable=no-member
+
+    def normalize_module(self, s, module=None, replace='module'):
+        if module is None:
+            module = type(self).__module__
+
+        return s.replace(module, replace)
+
+    def normalize(self, s):
+        return self.normalize_module(self.normalize_addr(s))
+
+    def assert_nstr_endswith(self, o, val):
+        s = str(o)
+        n = self.normalize(s)
+        self.assertTrue(n.endswith(val), (s, n))
+
+    def assert_nstr_startswith(self, o, val):
+        s = str(o)
+        n = self.normalize(s)
+        self.assertTrue(n.startswith(val), (s, n))
+
+
+
 class TestTimeout(gevent.Timeout):
     _expire_info = ''
 
@@ -178,7 +239,11 @@ class SubscriberCleanupMixin(object):
 
 
 class TestCase(TestCaseMetaClass("NewBase",
-                                 (SubscriberCleanupMixin, TimeAssertMixin, BaseTestCase,),
+                                 (SubscriberCleanupMixin,
+                                  TimeAssertMixin,
+                                  GreenletAssertMixin,
+                                  StringAssertMixin,
+                                  BaseTestCase,),
                                  {})):
     __timeout__ = params.LOCAL_TIMEOUT if not sysinfo.RUNNING_ON_CI else params.CI_TIMEOUT
 

src/gevent/tests/test__greenlet.py

@@ -18,7 +18,7 @@
 # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 # THE SOFTWARE.
-import re
+import functools
 import unittest
 
 import gevent.testing as greentest
@@ -32,6 +32,7 @@ from gevent.queue import Queue, Channel
 from gevent.testing.timing import AbstractGenericWaitTestCase
 from gevent.testing.timing import AbstractGenericGetTestCase
 from gevent.testing import timing
+from gevent.testing import ignores_leakcheck
 
 DELAY = timing.SMALL_TICK
 greentest.TestCase.error_fatal = False
@@ -90,7 +91,7 @@ class TestLink(greentest.TestCase):
         p2.link(q.put)
         p3.link(q.put)
         results = [q.get().get(), q.get().get(), q.get().get()]
-        assert sorted(results) == [101, 102, 103], results
+        self.assertEqual(sorted(results), [101, 102, 103], results)
 
 
 class TestUnlink(greentest.TestCase):
@@ -157,8 +158,10 @@ class LinksTestCase(greentest.TestCase):
         return event, queue
 
     def check_timed_out(self, event, queue):
-        assert with_timeout(DELAY, event.get, timeout_value=X) is X, repr(event.get())
-        assert with_timeout(DELAY, queue.get, timeout_value=X) is X, queue.get()
+        got = with_timeout(DELAY, event.get, timeout_value=X)
+        self.assertIs(got, X)
+        got = with_timeout(DELAY, queue.get, timeout_value=X)
+        self.assertIs(got, X)
 
 
 def return25():
@@ -233,12 +236,12 @@ class TestRaise_link(LinksTestCase):
         xxxxx = self.set_links_timeout(p.link_value)
 
         sleep(DELAY)
-        assert not p, p
+        self.assertFalse(p, p)
 
         self.assertRaises(ExpectedError, event.get)
         self.assertEqual(queue.get(), p)
         sleep(DELAY)
-        assert not callback_flag, callback_flag
+        self.assertFalse(callback_flag, callback_flag)
 
         self.check_timed_out(*xxxxx)
 
@@ -274,6 +277,7 @@ class TestStuff(greentest.TestCase):
         x.link(e)
         self.assertEqual(e.get(), 1)
 
+    @ignores_leakcheck
     def test_wait_error(self):
 
         def x():
@@ -284,8 +288,8 @@ class TestStuff(greentest.TestCase):
         self.assertRaises(ExpectedError, gevent.joinall, [x, y], raise_error=True)
         self.assertRaises(ExpectedError, gevent.joinall, [y], raise_error=True)
         x.join()
-    test_wait_error.ignore_leakcheck = True
 
+    @ignores_leakcheck
     def test_joinall_exception_order(self):
         # if there're several exceptions raised, the earliest one must be raised by joinall
         def first():
@@ -293,12 +297,10 @@ class TestStuff(greentest.TestCase):
             raise ExpectedError('first')
         a = gevent.spawn(first)
         b = gevent.spawn(lambda: getcurrent().throw(ExpectedError('second')))
-        try:
+        with self.assertRaisesRegex(ExpectedError, 'second'):
             gevent.joinall([a, b], raise_error=True)
-        except ExpectedError as ex:
-            assert 'second' in str(ex), repr(str(ex))
+
         gevent.joinall([a, b])
-    test_joinall_exception_order.ignore_leakcheck = True
 
     def test_joinall_count_raise_error(self):
         # When joinall is asked not to raise an error, the 'count' param still
@@ -314,12 +316,12 @@ class TestStuff(greentest.TestCase):
         raiser = gevent.spawn(raises_but_ignored)
 
         gevent.joinall([sleeper, raiser], raise_error=False, count=1)
-        assert_ready(raiser)
-        assert_not_ready(sleeper)
+        self.assert_greenlet_ready(raiser)
+        self.assert_greenlet_not_ready(sleeper)
 
         # Clean up our mess
         sleeper.kill()
-        assert_ready(sleeper)
+        self.assert_greenlet_ready(sleeper)
 
     def test_multiple_listeners_error(self):
         # if there was an error while calling a callback
@@ -408,8 +410,6 @@ class A(object):
     def method(self):
         pass
 
-hexobj = re.compile('-?0x[0123456789abcdef]+L?', re.I)
-
 class Subclass(gevent.Greenlet):
     pass
 
@@ -417,47 +417,38 @@ class TestStr(greentest.TestCase):
 
     def test_function(self):
         g = gevent.Greenlet.spawn(dummy_test_func)
-        self.assertTrue(hexobj.sub('X', str(g)).endswith('at X: dummy_test_func>'))
-        assert_not_ready(g)
+        self.assert_nstr_endswith(g, 'at X: dummy_test_func>')
+        self.assert_greenlet_not_ready(g)
         g.join()
-        assert_ready(g)
-        self.assertTrue(hexobj.sub('X', str(g)).endswith(' at X: dummy_test_func>'), str(g))
+        self.assert_greenlet_ready(g)
+        self.assert_nstr_endswith(g, 'at X: dummy_test_func>')
+
 
     def test_method(self):
         g = gevent.Greenlet.spawn(A().method)
-        str_g = hexobj.sub('X', str(g))
-        str_g = str_g.replace(__name__, 'module')
-        self.assertTrue(str_g.startswith('<Greenlet at X:'), str_g)
+        self.assert_nstr_startswith(g, '<Greenlet at X:')
+
         # Accessing the name to generate a minimal_ident will cause it to be included.
         getattr(g, 'name')
-        str_g = hexobj.sub('X', str(g))
-        str_g = str_g.replace(__name__, 'module')
-        self.assertTrue(str_g.startswith('<Greenlet "Greenlet-'), str_g)
+        self.assert_nstr_startswith(g, '<Greenlet "Greenlet-')
+
         # Assigning to the name changes it
         g.name = 'Foo'
-        str_g = hexobj.sub('X', str(g))
-        str_g = str_g.replace(__name__, 'module')
-        self.assertTrue(str_g.startswith('<Greenlet "Foo"'), str_g)
+        self.assert_nstr_startswith(g, '<Greenlet "Foo"')
 
-        self.assertTrue(str_g.endswith('at X: <bound method A.method of <module.A object at X>>>'))
-        assert_not_ready(g)
+        self.assert_nstr_endswith(g, 'at X: <bound method A.method of <module.A object at X>>>')
+        self.assert_greenlet_not_ready(g)
         g.join()
-        assert_ready(g)
-        str_g = hexobj.sub('X', str(g))
-        str_g = str_g.replace(__name__, 'module')
-        self.assertTrue(str_g.endswith('at X: <bound method A.method of <module.A object at X>>>'))
+        self.assert_greenlet_ready(g)
+        self.assert_nstr_endswith(g, 'at X: <bound method A.method of <module.A object at X>>>')
 
     def test_subclass(self):
         g = Subclass()
-        str_g = hexobj.sub('X', str(g))
-        str_g = str_g.replace(__name__, 'module')
-        self.assertTrue(str_g.startswith('<Subclass '), str_g)
-        self.assertTrue(str_g.endswith('at X: _run>'))
+        self.assert_nstr_startswith(g, '<Subclass ')
+        self.assert_nstr_endswith(g, 'at X: _run>')
 
         g = Subclass(None, 'question', answer=42)
-        str_g = hexobj.sub('X', str(g))
-        str_g = str_g.replace(__name__, 'module')
-        self.assertTrue(str_g.endswith(" at X: _run('question', answer=42)>"))
+        self.assert_nstr_endswith(g, " at X: _run('question', answer=42)>")
 
 
 class TestJoin(AbstractGenericWaitTestCase):
@@ -518,7 +509,7 @@ class TestBasic(greentest.TestCase):
 
         g = gevent.spawn_raw(f, 1, name='value')
         gevent.sleep(0.01)
-        assert not g
+        self.assertFalse(g)
         self.assertEqual(value[0], (1,))
         self.assertEqual(value[1], {'name': 'value'})
 
@@ -567,7 +558,13 @@ class TestBasic(greentest.TestCase):
         self.assertEqual(g.value, 5) # changed
         self.assertIsNone(g.exception, g)
 
-        self.assertTrue(link_test == [g] or greentest.RUNNING_ON_CI, link_test)  # changed
+        self._check_flaky_eq(link_test, g)
+
+    def _check_flaky_eq(self, link_test, g):
+        if not greentest.RUNNING_ON_CI:
+            # TODO: Change this to assertEqualFlakyRaceCondition and figure
+            # out what the CI issue is.
+            self.assertEqual(link_test, [g]) # changed
 
     def test_error_exit(self):
         link_test = []
@@ -591,91 +588,7 @@ class TestBasic(greentest.TestCase):
         self.assertFalse(g.successful())
         self.assertIsNone(g.value) # not changed
         self.assertEqual(g.exception.myattr, 5)
-
-        assert link_test == [g] or greentest.RUNNING_ON_APPVEYOR, link_test
-
-    def _assertKilled(self, g):
-        assert not g
-        assert g.dead
-        assert not g.started
-        assert g.ready()
-        assert g.successful(), (repr(g), g.value, g.exception)
-        assert isinstance(g.value, gevent.GreenletExit), (repr(g), g.value, g.exception)
-        assert g.exception is None
-
-    def assertKilled(self, g):
-        self._assertKilled(g)
-        gevent.sleep(0.01)
-        self._assertKilled(g)
-
-    def _test_kill(self, g, block):
-        g.kill(block=block)
-        if not block:
-            gevent.sleep(0.01)
-        self.assertKilled(g)
-        # kill second time must not hurt
-        g.kill(block=block)
-        self.assertKilled(g)
-
-    def _test_kill_not_started(self, block):
-        link_test = []
-        result = []
-        g = gevent.Greenlet(lambda: result.append(1))
-        g.link(link_test.append)
-        self._test_kill(g, block=block)
-        assert not result
-        assert link_test == [g]
-
-    def test_kill_not_started_block(self):
-        self._test_kill_not_started(block=True)
-
-    def test_kill_not_started_noblock(self):
-        self._test_kill_not_started(block=False)
-
-    def _test_kill_just_started(self, block):
-        result = []
-        link_test = []
-        g = gevent.Greenlet(lambda: result.append(1))
-        g.link(link_test.append)
-        g.start()
-        self._test_kill(g, block=block)
-        assert not result, result
-        assert link_test == [g]
-
-    def test_kill_just_started_block(self):
-        self._test_kill_just_started(block=True)
-
-    def test_kill_just_started_noblock(self):
-        self._test_kill_just_started(block=False)
-
-    def _test_kill_just_started_later(self, block):
-        result = []
-        link_test = []
-        g = gevent.Greenlet(lambda: result.append(1))
-        g.link(link_test.append)
-        g.start_later(1)
-        self._test_kill(g, block=block)
-        assert not result
-
-    def test_kill_just_started_later_block(self):
-        self._test_kill_just_started_later(block=True)
-
-    def test_kill_just_started_later_noblock(self):
-        self._test_kill_just_started_later(block=False)
-
-    def _test_kill_running(self, block):
-        link_test = []
-        g = gevent.spawn(gevent.sleep, 10)
-        g.link(link_test.append)
-        self._test_kill(g, block=block)
-        gevent.sleep(0.01)
-        assert link_test == [g]
-
-    def test_kill_running_block(self):
-        self._test_kill_running(block=True)
-
-    def test_kill_running_noblock(self):
-        self._test_kill_running(block=False)
+        self._check_flaky_eq(link_test, g)
 
     def test_exc_info_no_error(self):
         # Before running
@@ -776,34 +689,120 @@ class TestBasic(greentest.TestCase):
             g.kill()
 
 
+class TestKill(greentest.TestCase):
 
+    def __assertKilled(self, g):
+        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)
 
-class TestStart(greentest.TestCase):
+    def assertKilled(self, g):
+        self.__assertKilled(g)
+        gevent.sleep(0.01) # spin the loop to make sure it doesn't run.
+        self.__assertKilled(g)
+
+    def __kill_greenlet(self, g, block, killall):
+        if killall:
+            killer = functools.partial(gevent.killall, [g], block=block)
+        else:
+            killer = functools.partial(g.kill, 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)
+        # kill second time must not hurt
+        killer()
+        self.assertKilled(g)
 
-    def test(self):
-        g = gevent.spawn(gevent.sleep, 0.01)
-        assert g.started
-        assert not g.dead
-        g.start()
-        assert g.started
-        assert not g.dead
-        g.join()
-        assert not g.started
-        assert g.dead
+    @staticmethod
+    def _run_in_greenlet(result_collector):
+        result_collector.append(1)
+
+    def _start_greenlet(self, g):
+        """
+        Subclasses should override. This doesn't actually start a greenlet.
+        """
+
+    _after_kill_greenlet = _start_greenlet
+
+
+    def _do_test(self, block, killall):
+        link_test = []
+        result = []
+        g = gevent.Greenlet(self._run_in_greenlet, result)
+        g.link(link_test.append)
+
+        self._start_greenlet(g)
+
+        self.__kill_greenlet(g, block, killall)
+
+        self._after_kill_greenlet(g)
+
+        self.assertFalse(result)
+        self.assertEqual(link_test, [g])
+
+    def test_block(self):
+        self._do_test(block=True, killall=False)
+
+    def test_non_block(self):
+        self._do_test(block=False, killall=False)
+
+    def test_block_killal(self):
+        self._do_test(block=True, killall=True)
+
+    def test_non_block_killal(self):
+        self._do_test(block=False, killall=True)
+
+
+class TestKillAfterStart(TestKill):
+
+    def _start_greenlet(self, g):
         g.start()
-        assert not g.started
-        assert g.dead
 
+class TestKillAfterStartLater(TestKill):
+
+    def _start_greenlet(self, g):
+        g.start_later(timing.LARGE_TICK)
+
+class TestKillWhileRunning(TestKill):
+
+    @staticmethod
+    def _run_in_greenlet(result_collector):
+        gevent.sleep(10)
+        # The above should die with the GreenletExit exception,
+        # so this should never run
+        TestKill._run_in_greenlet(result_collector)
 
-def assert_ready(g):
-    assert g.dead, g
-    assert g.ready(), g
-    assert not bool(g), g
+    def _after_kill_greenlet(self, g):
+        TestKill._after_kill_greenlet(self, g)
+        gevent.sleep(0.01)
+
+class TestKillallRawGreenlet(greentest.TestCase):
+
+    def test_killall_raw(self):
+        g = gevent.spawn_raw(lambda: 1)
+        gevent.killall([g])
 
+class TestStart(greentest.TestCase):
 
-def assert_not_ready(g):
-    assert not g.dead, g
-    assert not g.ready(), g
+    def test_start(self):
+        g = gevent.spawn(gevent.sleep, timing.SMALL_TICK)
+        self.assert_greenlet_spawned(g)
+
+        g.start()
+        self.assert_greenlet_started(g)
+
+        g.join()
+        self.assert_greenlet_finished(g)
+
+        # cannot start again
+        g.start()
+        self.assert_greenlet_finished(g)
 
 
 class TestRef(greentest.TestCase):
@@ -814,12 +813,12 @@ class TestRef(greentest.TestCase):
         gevent.Greenlet()
 
     def test_kill_scheduled(self):
-        gevent.spawn(gevent.sleep, 10).kill()
+        gevent.spawn(gevent.sleep, timing.LARGE_TICK).kill()
 
     def test_kill_started(self):
-        g = gevent.spawn(gevent.sleep, 10)
+        g = gevent.spawn(gevent.sleep, timing.LARGE_TICK)
         try:
-            gevent.sleep(0.001)
+            gevent.sleep(timing.SMALLEST_RELIABLE_DELAY)
         finally:
             g.kill()
 

src/gevent/tests/test__greenletset.py

@@ -138,7 +138,7 @@ class Test(greentest.TestCase):
         for g in s:
             assert g.dead
 
-    def test_killall_iterable_argument_timeout(self):
+    def test_killall_iterable_argument_timeout_not_started(self):
         def f():
             try:
                 gevent.sleep(1.5)
@@ -149,6 +149,25 @@ class Test(greentest.TestCase):
         s = set()
         s.add(p1)
         s.add(p2)
+        gevent.killall(s, timeout=0.5)
+
+        for g in s:
+            self.assertTrue(g.dead, g)
+
+    def test_killall_iterable_argument_timeout_started(self):
+        def f():
+            try:
+                gevent.sleep(1.5)
+            except: # pylint:disable=bare-except
+                gevent.sleep(1)
+        p1 = GreenletSubclass.spawn(f)
+        p2 = GreenletSubclass.spawn(f)
+
+        s = set()
+        s.add(p1)
+        s.add(p2)
+        # Get them both running.
+        gevent.sleep(timing.SMALLEST_RELIABLE_DELAY)
         with self.assertRaises(Timeout):
             gevent.killall(s, timeout=0.5)
 

src/gevent/tests/test__hub.py

@@ -159,6 +159,7 @@ class TestPeriodicMonitoringThread(greentest.TestCase):
         gevent.config.monitor_thread = self.monitor_thread
         self.monitored_hubs = None
         self._reset_hub()
+        super(TestPeriodicMonitoringThread, self).tearDown()
 
     def _monitor(self, hub):
         self.monitor_fired += 1

src/gevent/tests/test__issue330.py

@@ -65,7 +65,8 @@ class TestSwitch(greentest.TestCase):
             self.caught = e
 
     def test_kill_exception(self):
-        # Killing with gevent.kill gets the right exception
+        # Killing with gevent.kill gets the right exception,
+        # and we can pass exception objects, not just exception classes.
 
         g = gevent.spawn(self.catcher)
         g.start()

src/gevent/tests/test__pywsgi.py

@@ -17,7 +17,7 @@
 # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 # THE SOFTWARE.
-# pylint: disable=too-many-lines,unused-argument
+# pylint: disable=too-many-lines,unused-argument,too-many-ancestors
 from __future__ import print_function
 
 from gevent import monkey

src/gevent/tests/test__server_pywsgi.py

@@ -93,13 +93,13 @@ class TestDefaultSpawn(test__server.TestDefaultSpawn):
 class TestSSLSocketNotAllowed(test__server.TestSSLSocketNotAllowed):
     Settings = Settings
 
-class TestRawSpawn(test__server.TestRawSpawn):
+class TestRawSpawn(test__server.TestRawSpawn): # pylint:disable=too-many-ancestors
     Settings = Settings
 
 class TestSSLGetCertificate(test__server.TestSSLGetCertificate):
     Settings = Settings
 
-class TestPoolSpawn(test__server.TestPoolSpawn):
+class TestPoolSpawn(test__server.TestPoolSpawn): # pylint:disable=too-many-ancestors
     Settings = Settings
 
 if __name__ == '__main__':

src/gevent/tests/test__threadpool_executor_patched.py

@@ -9,7 +9,7 @@ import gevent.testing as greentest
 from . import test__threadpool
 
 
-class TestPatchedTPE(test__threadpool.TestTPE):
+class TestPatchedTPE(test__threadpool.TestTPE): # pylint:disable=too-many-ancestors
     MONKEY_PATCHED = True