classification
Title: asyncio: Future.set_result() called on cancelled Future raises asyncio.futures.InvalidStateError
Type: Stage: resolved
Components: Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: python-dev, ryder.lewis, vstinner
Priority: normal Keywords:

Created on 2014-06-30 14:05 by vstinner, last changed 2014-07-05 13:32 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
bug.py vstinner, 2014-06-30 14:05
Messages (6)
msg221961 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-06-30 14:05
Ok, I found a way to reproduce the error InvalidStateError in asyncio. I'm not sure that it's the same the error in #21447.

Output of attached bug.py in debug mode:
---
Exception in callback Future.set_result(None)
handle: <TimerHandle when=79580.878306285 Future.set_result(None)>
source_traceback: Object created at (most recent call last):
  File "/home/haypo/bug.py", line 11, in <module>
    loop.run_until_complete(task2)
  File "/home/haypo/prog/python/default/Lib/asyncio/base_events.py", line 239, in run_until_complete
    self.run_forever()
  File "/home/haypo/prog/python/default/Lib/asyncio/base_events.py", line 212, in run_forever
    self._run_once()
  File "/home/haypo/prog/python/default/Lib/asyncio/base_events.py", line 912, in _run_once
    handle._run()
  File "/home/haypo/prog/python/default/Lib/asyncio/events.py", line 96, in _run
    self._callback(*self._args)
  File "/home/haypo/prog/python/default/Lib/asyncio/tasks.py", line 241, in _step
    result = next(coro)
  File "/home/haypo/prog/python/default/Lib/asyncio/coroutines.py", line 72, in __next__
    return next(self.gen)
  File "/home/haypo/prog/python/default/Lib/asyncio/tasks.py", line 487, in sleep
    h = future._loop.call_later(delay, future.set_result, result)
Traceback (most recent call last):
  File "/home/haypo/prog/python/default/Lib/asyncio/events.py", line 96, in _run
    self._callback(*self._args)
  File "/home/haypo/prog/python/default/Lib/asyncio/futures.py", line 326, in set_result
    raise InvalidStateError('{}: {!r}'.format(self._state, self))
asyncio.futures.InvalidStateError: CANCELLED: <Future cancelled>
---

The fix is to replace the following line of sleep():
---
    h = future._loop.call_later(delay, future.set_result, result)
---

with:
---
    def maybe_set_result(future, result):
        if not future.cancelled():
            future.set_result(result)
    h = future._loop.call_later(delay, maybe_set_result, future, result)
---

This generic issue was already discussed there:
https://groups.google.com/forum/?fromgroups#!searchin/python-tulip/set_result$20InvalidStateError/python-tulip/T1sxLqjuoVY/YghF-YsgosgJ

A patch was also proposed:
https://codereview.appspot.com/69870048/
msg221994 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-06-30 23:33
I see two options to fix this issue:

- add an optional parameter to set_result() to do nothing if the future is cancelled
- add a method (public or private) to set a result or do nothing if the future is cancelled

Patch "Add ignore_cancelled and ignore_done to Future.set_result()" (for Tulip):
http://codereview.appspot.com/109340043

Patch "Add Future._maybe_set_result()":
http://codereview.appspot.com/108300043

I prefer the second patch because it doesn't touch the public API and it is shorter.

Note: the first patch contains unrelated changes, like checking fut.cancelled() instead of fut.done().

"_maybe_set_result()" is not a good name. Other suggestions: "_set_result_except_cancelled", ""_set_result_ignore_cancelled".
msg221995 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-06-30 23:34
In https://codereview.appspot.com/69870048/ Guido proposed to test to replace:
   self._loop.call_soon(waiter.set_result, None)
with:
   if not waiter.cancelled():
      waiter.set_result(None)
msg221996 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-06-30 23:45
> "_maybe_set_result()" is not a good name. Other suggestions: "_set_result_except_cancelled", ""_set_result_ignore_cancelled".

I read again the mail thread and Guido proposed the nice name _set_result_unless_cancelled() which is very explicit. I updated my patch:
http://codereview.appspot.com/108300043
msg222357 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-07-05 13:31
New changeset d7e4efd5e279 by Victor Stinner in branch '3.4':
Closes #21886, #21447: Fix a race condition in asyncio when setting the result
http://hg.python.org/cpython/rev/d7e4efd5e279

New changeset 50c995bdc00a by Victor Stinner in branch 'default':
(Merge 3.4) Closes #21886, #21447: Fix a race condition in asyncio when setting
http://hg.python.org/cpython/rev/50c995bdc00a
msg222359 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-07-05 13:32
Fix commited to Tulip (4655ef2d9f43), Python 3.4 and 3.5.
History
Date User Action Args
2014-07-05 13:32:06vstinnersetmessages: + msg222359
2014-07-05 13:31:25python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg222357

resolution: fixed
stage: resolved
2014-06-30 23:45:39vstinnersetmessages: + msg221996
2014-06-30 23:34:59vstinnersetmessages: + msg221995
2014-06-30 23:33:08vstinnersetmessages: + msg221994
2014-06-30 14:15:03ryder.lewissetnosy: + ryder.lewis
2014-06-30 14:05:18vstinnercreate