Fix python bug 13502

http://bugs.python.org/issue13502

A race condition in Event that caused an improper return value if
greenlets were waiting while the event was set, but then immediately
cleared.

This was fixed in Python 3 but not (yet?) backported to Python 2.7, even
though the bug identifies that as a target.

Not only does this behaviour seem more obviously correct to me, it's
more future-proof: user share of Python 2 should be diminishing while
Python 3 share (with the correct behaviour) grows.

changelog.rst

@@ -13,6 +13,13 @@
   None). The ``acquire`` method also raises the same :exc:`ValueError`
   exceptions that the standard library does for invalid parameters.
   Reported in :issue:`750` by Joy Zheng.
+- Fix a race condition in :class:`~gevent.event.Event` that
+  made it return ``False`` when the event was set and cleared by the
+  same greenlet before allowing a switch to the waiting greenlets.
+  (Found by the 3.4 and 3.5 standard library test suites; the same as
+  Python `bug 13502`_).
+
+.. _bug 13502: http://bugs.python.org/issue13502
 
 1.1rc5 (Feb 24, 2016)
 =====================

gevent/event.py

@@ -77,6 +77,7 @@ class _AbstractLinkable(object):
         # The core of the wait implementation, handling
         # switching and linking. If *catch* is set to (),
         # a timeout that elapses will be allowed to be raised.
+        # Returns a true value if the wait succeeded without timing out.
         switch = getcurrent().switch
         self.rawlink(switch)
         try:
@@ -86,22 +87,27 @@ class _AbstractLinkable(object):
                     result = self.hub.switch()
                     if result is not self: # pragma: no cover
                         raise InvalidSwitchError('Invalid switch into Event.wait(): %r' % (result, ))
+                    return True
                 except catch as ex:
                     if ex is not timer:
                         raise
+                    # test_set_and_clear and test_timeout in test_threading
+                    # rely on the exact return values, not just truthish-ness
+                    return False
             finally:
                 timer.cancel()
         finally:
             self.unlink(switch)
 
-    _wait_return_value = None
+    def _wait_return_value(self, waited, wait_success):
+        return None
 
     def _wait(self, timeout=None):
         if self.ready():
-            return getattr(self, self._wait_return_value)
+            return self._wait_return_value(False, False)
 
-        self._wait_core(timeout)
-        return getattr(self, self._wait_return_value)
+        gotit = self._wait_core(timeout)
+        return self._wait_return_value(True, gotit)
 
 
 class Event(_AbstractLinkable):
@@ -153,7 +159,17 @@ class Event(_AbstractLinkable):
         """
         self._flag = False
 
-    _wait_return_value = '_flag'
+    def _wait_return_value(self, waited, gotit):
+        # To avoid the race condition outlined in http://bugs.python.org/issue13502,
+        # if we had to wait, then we need to return whether or not
+        # the condition got changed. Otherwise we simply echo
+        # the current state of the flag (which should be true)
+        if not waited:
+            flag = self._flag
+            assert flag, "if we didn't wait we should already be set"
+            return flag
+
+        return gotit
 
     def wait(self, timeout=None):
         """
@@ -167,8 +183,10 @@ class Event(_AbstractLinkable):
         floating point number specifying a timeout for the operation in seconds
         (or fractions thereof).
 
-        :return: The value of the internal flag (``True`` or ``False``).
-           (If no timeout was given, the only possible return value is ``True``.)
+        :return: This method returns true if and only if the internal flag has been set to
+            true, either before the wait call or after the wait starts, so it will
+            always return ``True`` except if a timeout is given and the operation
+            times out.
         """
         return self._wait(timeout)
 
@@ -340,7 +358,10 @@ class AsyncResult(_AbstractLinkable):
         """
         return self.get(block=False)
 
-    _wait_return_value = 'value'
+    def _wait_return_value(self, waited, gotit):
+        # Always return the value. Since this is a one-shot event,
+        # no race condition should reset it.
+        return self.value
 
     def wait(self, timeout=None):
         """Block until the instance is ready.

greentest/3.4/test_threading.py

@@ -1082,10 +1082,6 @@ class CRLockTests(lock_tests.RLockTests):
 class EventTests(lock_tests.EventTests):
     eventtype = staticmethod(threading.Event)
 
-    @unittest.skip("gevent may/not suffer from Python bug 13502")
-    def test_set_and_clear(self):
-        pass
-
     @unittest.skip("not on gevent")
     def test_reset_internal_locks(self):
         pass

greentest/3.5/test_threading.py

@@ -1090,10 +1090,6 @@ class CRLockTests(lock_tests.RLockTests):
 class EventTests(lock_tests.EventTests):
     eventtype = staticmethod(threading.Event)
 
-    @unittest.skip("gevent may/not suffer from Python bug 13502")
-    def test_set_and_clear(self):
-        pass
-
     @unittest.skip("not on gevent")
     def test_reset_internal_locks(self):
         pass