Don't immediately run links added to an Event/AsyncResult while it is running...

Don't immediately run links added to an Event/AsyncResult while it is running callbacks. Fixes #771.

gevent/event.py

@@ -63,15 +63,14 @@ class _AbstractLinkable(object):
         # Actually call the notification callbacks. Those callbacks in todo that are
         # still in _links are called. This method is careful to avoid iterating
         # over self._links, because links could be added or removed while this
-        # method runs. For the same reason, we loop, checking for new items in
-        # _links.
+        # method runs. Only links present when this method begins running
+        # will be called; if a callback adds a new link, it will not run
+        # until the next time notify_links is activated
 
         # We don't need to capture self._links as todo when establishing
         # this callback; any links removed between now and then are handled
         # by the `if` below; any links added are also grabbed
         todo = set(self._links)
-        done = set()
-        while todo:
         for link in todo:
             # check that link was not notified yet and was not removed by the client
             # We have to do this here, and not as part of the 'for' statement because
@@ -81,11 +80,6 @@ class _AbstractLinkable(object):
                     link(self)
                 except:
                     self.hub.handle_error((link, self), *sys.exc_info())
-            # Mark everything done
-            done.update(todo)
-            # Anything extra now in self._links but not yet done, loop
-            # again for
-            todo = self._links - done
 
     def _wait_core(self, timeout, catch=Timeout):
         # The core of the wait implementation, handling

greentest/test__event.py

@@ -20,6 +20,39 @@ class TestWaitEvent(greentest.GenericWaitTestCase):
     def wait(self, timeout):
         gevent.wait([Event()], timeout=timeout)
 
+    def test_set_during_wait(self):
+        # https://github.com/gevent/gevent/issues/771
+        # broke in the refactoring. we must not add new links
+        # while we're running the callback
+
+        event = Event()
+
+        def setter():
+            event.set()
+
+        def waiter():
+            s = gevent.spawn(setter)
+            # let the setter set() the event;
+            # when this method returns we'll be running in the Event._notify_links callback
+            # (that is, it switched to us)
+            res = event.wait()
+            self.assertTrue(res)
+            self.assertTrue(event.ready())
+            s.join() # make sure it's dead
+            # Clear the event. Now we can't wait for the event without
+            # another set to happen.
+            event.clear()
+            self.assertFalse(event.ready())
+
+            # Before the bug fix, this would return "immediately" with
+            # event in the result list, because the _notify_links loop would
+            # immediately add the waiter and call it
+            o = gevent.wait((event,), timeout=0.01)
+            self.assertFalse(event.ready())
+            self.assertFalse(event in o, o)
+
+        gevent.spawn(waiter).join()
+
 
 class TestAsyncResultWait(greentest.GenericWaitTestCase):