classification
Title: Deprecate task.set_result() and task.set_exception()
Type: behavior Stage: resolved
Components: asyncio, Library (Lib) Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Ilya.Kulakov, asvetlov, chris.jerdonek, yselivanov
Priority: normal Keywords: patch

Created on 2017-12-18 10:59 by asvetlov, last changed 2019-02-19 20:43 by chris.jerdonek. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 4923 merged yselivanov, 2017-12-19 05:39
Messages (17)
msg308542 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2017-12-18 10:59
Task result is given from coroutine execution, explicit modification of the value should be deprecated and eventually removed.
msg308579 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2017-12-18 18:05
+1.
msg308609 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2017-12-19 03:39
Actually, I don't think we need a deprecation period.  Both methods are completely unusable now.

Given:

  async def foo():
    await asyncio.sleep(1)
    print('AAAAA')
    return 42


1. Let's try to use Task.set_result():

  async def main():
    f = asyncio.create_task(foo())
    f.set_result(123)
    result = await f
    print(result)

  asyncio.run(main())

This will print "123", but will give us an "AssertionError: _step(): already done: <Task finished coro=<foo() running at t.py:4> result=123> None" logged.


2. Let's try set_result + a sleep:

  async def main():
    f = asyncio.create_task(foo())
    await asyncio.sleep(2)
    f.set_result(123)
    result = await f
    print(result)


  asyncio.run(main())

This just crashes with an InvalidStateError.

3. Same happens with set_exception().


All in all, deprecation periods are useful when there's some working and useful functionality that has to be removed in the future.  In this case there's no working and no useful functionality.

So let's just make Task.set_result() and Task.set_exception() raise NotImplementedError.
msg308613 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2017-12-19 04:50
About the only use case I can come up with is subclassing asyncio.Task and overriding set_result and set_exception implementations.  This use case has been broken (unintentionally) in Python 3.6, when we first implemented Task in _asynciomodule.c.  C Task doesn't try to resolve 'self.set_result()' or 'self.set_exception()'.  It calls the C "super" implementations instead.
msg309037 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2017-12-25 15:48
New changeset 0cf16f9ea014b17d398ee3971d4976c698533318 by Yury Selivanov in branch 'master':
bpo-32363: Disable Task.set_exception() and Task.set_result() (#4923)
https://github.com/python/cpython/commit/0cf16f9ea014b17d398ee3971d4976c698533318
msg309198 - (view) Author: Ilya Kulakov (Ilya.Kulakov) * Date: 2017-12-29 21:55
Andrew, Yury

I test my lib against dev versions of Python and recently got an error in one of the tests due to the deprecation.

I do not argue the reason behind removing this methods, but Task.set_exception was working for me in tests: https://github.com/Kentzo/async_app/blob/250ee7a05d2af8035806ce1d86f57d0f00283db0/tests/test_utils.py#L73-L91

My use case was outside control of otherwise unconditionally blocking task (for tests only). What replacement (if any) would you suggest?
msg309199 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2017-12-29 21:58
> My use case was outside control of otherwise unconditionally blocking task (for tests only). What replacement (if any) would you suggest?


Use Task.cancel() or use a Queue to communicate with the task.  Your test code was working, but ultimately was creating an unexpected (and not officially documented/supported) situation for the task.
msg309201 - (view) Author: Ilya Kulakov (Ilya.Kulakov) * Date: 2017-12-29 22:03
cancel will work in my case, but it's somewhat limited.

In other hand it's probably a job of a testing library to provide an utility function.
msg336000 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2019-02-19 19:08
> Use Task.cancel() or use a Queue to communicate with the task.

One reason why Task.cancel() is an incomplete replacement for Task.set_exception() is that you don't have an easy way to communicate why the task was ended.

With set_exception() and set_result(), you could. Task.cancel(), though, doesn't let you e.g. specify a CancelledError subclass or "reason" string (see the related bpo-35674, "Add argument to .cancel() of Task and Future").
msg336001 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2019-02-19 19:10
Correction: that should have been bpo-31033.
msg336002 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2019-02-19 19:14
> One reason why Task.cancel() is an incomplete replacement for Task.set_exception() is that you don't have an easy way to communicate why the task was ended.


The result of the Task is bound to the result of the coroutine it wraps.  If the coroutine raises an exception -- that's the exception of the Task object.  If the coroutine returns a value -- that's the result value of the Task object.  If the coroutine is cancelled via the "Task.cancel()" call -- asyncio.CancelledError is likely the result of the Task.

Key rule: the wrapped coroutine dictates the result of the Task, not the other way around.  The Task can signal cancellation, but even then, the final result is up to the coroutine.

There's no clear reason to complicate the Task<->coroutine relationship by allowing to inject arbitrary exceptions into running coroutines.
msg336003 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2019-02-19 19:17
A second reason why Task.cancel() seems to be an incomplete replacement:

Task.set_exception() and Task.set_result() both give you a way to unconditionally end a task. With cancel() though, the docs say, "Task.cancel() does not guarantee that the Task will be cancelled." [1]

The reason you might want to unconditionally end a task is if e.g. you already called Task.cancel() and it is still running after waiting a certain amount of time.

[1]: https://docs.python.org/3/library/asyncio-task.html#asyncio.Task.cancel
msg336005 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2019-02-19 19:29
> There's no clear reason to complicate the Task<->coroutine relationship by allowing to inject arbitrary exceptions into running coroutines.

My comment was more about CancelledError rather than arbitrary exceptions. You didn't reply to the part of my response saying that Task.cancel() doesn't let you communicate why the task was ended, which is something the removed API's let you do.
msg336009 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2019-02-19 19:54
>> There's no clear reason to complicate the Task<->coroutine relationship by allowing to inject arbitrary exceptions into running coroutines.

> My comment was more about CancelledError rather than arbitrary exceptions. You didn't reply to the part of my response saying that Task.cancel() doesn't let you communicate why the task was ended, [..]

I didn't reply because you didn't clearly articulate why the cancellation reason is important :)  AFAIK no other async framework allows you to do this.  So what are those real-world cases that can explain why asyncio should support this functionality?

> [..] which is something the removed API's let you do.

Unfortunately they didn't.  They never worked properly; IIRC Task.set_exception had never worked at all.  The fact they were available at all was a simple oversight.

> Task.set_exception() and Task.set_result() both give you a way to unconditionally end a task. With cancel() though, the docs say, "Task.cancel() does not guarantee that the Task will be cancelled." [1]

Yes, because Task can only signal its coroutine that it was requested to be cancelled.  The coroutine may choose to ignore that by swallowing CancelledError.

> The reason you might want to unconditionally end a task is if e.g. you already called Task.cancel() and it is still running after waiting a certain amount of time.

Well, if you want to unconditionally end tasks you shouldn't write coroutines that ignore CancelledErrors.  If you do, then you clearly *do not want* the coroutine to be cancelled, and thus there's no use case for set_exception.

Finally, asyncio has queues, which can be used to build two-way communication channels between coroutines and implement whatever advanced cancellation mechanism you want.
msg336013 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2019-02-19 20:07
> Well, if you want to unconditionally end tasks you shouldn't write coroutines that ignore CancelledErrors.

Well, of course. But code can have bugs, and maybe you didn't write the coroutine because it's from a library that you don't control. In that case, you should still be able to end the task. Also, maybe the coroutine doesn't have a bug but the cancellation is just taking longer than you have time for, so you want to end it early.

> So what are those real-world cases that can explain why asyncio should support this functionality?

The case I had in mind was the one I referenced above -- being able to distinguish a normal CancelledError from one where you had interrupt the cancellation (i.e. cancellation timing out). I would like the caller to be able to know when the latter are happening.
msg336014 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2019-02-19 20:22
I agree with Yuri.

`Task.set_exception()` (let's assume it works) is very dangerous: if cancellation exception doesn't bubble up from coroutine code there is a very high chance to get broken invariants and not-released resources.

The same situation is possible with classic threads: killing a thread without unwinding a call stack leads to locked mutexes etc.

Regarding distinguishing explicit cancellation from timeout exhausting: it can be done with current asyncio design by using a flag. Take a look on async_timeout context manager, __aexit__() implementation: https://github.com/aio-libs/async-timeout/blob/master/async_timeout/__init__.py#L88-L97
msg336019 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2019-02-19 20:43
To stop the discussion from happening in two places (sorry, Yury), I started a broader discussion on Async-sig with thread starting here: https://mail.python.org/pipermail/async-sig/2019-February/000548.html
History
Date User Action Args
2019-02-19 20:43:46chris.jerdoneksetmessages: + msg336019
2019-02-19 20:22:58asvetlovsetmessages: + msg336014
2019-02-19 20:07:03chris.jerdoneksetmessages: + msg336013
2019-02-19 19:54:20yselivanovsetmessages: + msg336009
2019-02-19 19:29:08chris.jerdoneksetmessages: + msg336005
2019-02-19 19:17:56chris.jerdoneksetmessages: + msg336003
2019-02-19 19:14:14yselivanovsetmessages: + msg336002
2019-02-19 19:10:01chris.jerdoneksetmessages: + msg336001
2019-02-19 19:08:53chris.jerdoneksetnosy: + chris.jerdonek
messages: + msg336000
2017-12-29 22:03:10Ilya.Kulakovsetmessages: + msg309201
2017-12-29 21:58:00yselivanovsetmessages: + msg309199
2017-12-29 21:55:53Ilya.Kulakovsetnosy: + Ilya.Kulakov
messages: + msg309198
2017-12-25 15:48:39yselivanovsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2017-12-25 15:48:17yselivanovsetmessages: + msg309037
2017-12-19 05:39:55yselivanovsetkeywords: + patch
stage: patch review
pull_requests: + pull_request4818
2017-12-19 04:50:03yselivanovsetmessages: + msg308613
2017-12-19 03:39:53yselivanovsetmessages: + msg308609
2017-12-18 18:05:48yselivanovsetmessages: + msg308579
2017-12-18 10:59:23asvetlovcreate