InvalidStateError after using run_until_complete() with timeout about asyncio

[GoogleCodeExporter]

The following patch seems to fix it.

diff -r cde99cf41775 tulip/base_events.py
--- a/tulip/base_events.py      Fri May 31 23:27:40 2013 +0100
+++ b/tulip/base_events.py      Sat Jun 01 17:13:00 2013 +0100
@@ -148,6 +148,7 @@
             handle = self.call_later(timeout, stop_loop)
             self.run_forever()
             handle.cancel()
+            future.remove_done_callback(_raise_stop_error)

         if handle_called:
             raise futures.TimeoutError

Original comment by [email protected] on 1 Jun 2013 at 4:17

[GoogleCodeExporter]

Can you come up with a test that reproduces the problem?

FWIW, we've struggled with this function quite a bit.  The current 
implementation looks odd.  Also it seems to be missing tests for everything 
except a badly typed argument.

Original comment by [email protected] on 3 Jun 2013 at 12:16

[GoogleCodeExporter]

Please see possible solution: http://codereview.appspot.com/9957043

This adds some tests for run_until_complete(), including one that proves your 
fix works.  I still think that run_until_complete() is too clumsy though.

Original comment by [email protected] on 3 Jun 2013 at 12:28

[GoogleCodeExporter]

I think some confusion is caused by the way that cancel() does different things 
to the done callbacks of Futures and Tasks.

For a Future cancel() prevents the done callbacks from running.  But for a 
Task, CancelledError gets thrown in to the coroutine, which later gets caught, 
causing set_exception() to be called, and the callbacks to be invoked.

Original comment by [email protected] on 3 Jun 2013 at 7:27

[GoogleCodeExporter]

 > I think some confusion is caused by the way that cancel() does different
> things to the done callbacks of Futures and Tasks.

There's definitely confusion. :-)

> For a Future cancel() prevents the done callbacks from running.

Wrong. :-) From futures.py:

    def cancel(self):
        """..."""
        if self._state != _PENDING:
            return False
        self._state = _CANCELLED
        self._schedule_callbacks()
        return True

This is consistent with PEP 3148.

> But for a
> Task, CancelledError gets thrown in to the coroutine, which later gets
> caught, causing set_exception() to be called, and the callbacks to be
> invoked.

Presumably something else could monitor a regular future's
cancellation too, through the done-callback (but this depends
completely on what the future represents).

Original comment by [email protected] on 3 Jun 2013 at 3:39

[GoogleCodeExporter]

i think confusion is caused by the difference between 'timeout' and 'cancelled'.
timeout does not stop task execution, so it eventually could complete.

Original comment by [email protected] on 3 Jun 2013 at 4:29

[GoogleCodeExporter]

here is test, it passes with Richard's patch.

    def test_run_until_complete_concurrent_stop(self):
        a = tasks.sleep(0.2)
        self.assertRaises(
            futures.TimeoutError,
            self.loop.run_until_complete, a, 0.1)

        b = tasks.sleep(1.0)
        self.assertRaises(
            futures.TimeoutError,
            self.loop.run_until_complete, b, 0.2)

Original comment by [email protected] on 3 Jun 2013 at 4:39

[GoogleCodeExporter]

> This is consistent with PEP 3148.

Ah.  I should probably read that some time...

Original comment by [email protected] on 3 Jun 2013 at 5:22

[GoogleCodeExporter]

Fixed by revision c1f04ef931a4.

Original comment by [email protected] on 7 Jun 2013 at 6:27

[GoogleCodeExporter]

Original comment by [email protected] on 9 Jun 2013 at 11:56

• Changed state: Fixed