classification
Title: Add names to asyncio tasks
Type: enhancement Stage: resolved
Components: asyncio Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: alex.gronholm, asvetlov, benjamin.peterson, eric.snow, yselivanov
Priority: normal Keywords: patch

Created on 2018-07-29 09:25 by alex.gronholm, last changed 2018-08-14 04:32 by benjamin.peterson. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 8547 merged python-dev, 2018-07-29 11:41
PR 8717 merged alex.gronholm, 2018-08-09 16:19
PR 8759 merged benjamin.peterson, 2018-08-14 04:15
Messages (21)
msg322620 - (view) Author: Alex Grönholm (alex.gronholm) * Date: 2018-07-29 09:25
Having names on tasks helps tremendously when something goes wrong in a complex asyncio application. Threads have names and even trio has the ability to name its tasks. This would also greatly benefit PyCharm's concurrency visualization: https://www.jetbrains.com/help/pycharm/thread-concurrency-visualization.html#asyncio
msg323302 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-08-08 21:06
New changeset cca4eec3c0a67cbfeaf09182ea6c097a94891ff6 by Yury Selivanov (Alex Grönholm) in branch 'master':
bpo-34270: Make it possible to name asyncio tasks (GH-8547)
https://github.com/python/cpython/commit/cca4eec3c0a67cbfeaf09182ea6c097a94891ff6
msg323303 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-08-08 21:08
Thank you for the contribution!
msg323324 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-08-09 14:41
FWIW, the C implementation of Task.__init__ is not exactly equivalent to the Python implementation (nor to both the C and Python implementation of Task.set_name).  In the C impl of Task.__init__ the provided name is used as-is if it's an instance of str:

   (_asyncio_Task___init___impl() in Modules/_asynciomodule.c)

   if (name == Py_None) {
       name = PyUnicode_FromFormat("Task-%" PRIu64, ++task_name_counter);
   } else if (!PyUnicode_Check(name)) {
       name = PyObject_Str(name);
   } else {
       Py_INCREF(name);
   }

One of the following should happen, right?

1. fix the Python implementation of Task.__init__() and both impl of Task.set_name()
2. change the check to PyUnicode_CheckExact()
3. remove the special-case (i.e. change the C impl to match the Python impl)

p.s. Sorry I did not notice this before it got committed. :/
msg323325 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-08-09 14:46
> 2. change the check to PyUnicode_CheckExact()

I'd be OK with this, but why is this important?
msg323326 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-08-09 14:47
As a side note, Alex, what do you think about appending coroutine's name to Task's name if the latter is autogenerated?
msg323327 - (view) Author: Alex Grönholm (alex.gronholm) * Date: 2018-08-09 14:51
I also couldn't figure out yet why PyUnicode_Check() was necessary in the first place. Doesn't PyObject_Str() just increment the refcount if the argument is already a string?

Eric, please explain why these changes should be done.
msg323328 - (view) Author: Alex Grönholm (alex.gronholm) * Date: 2018-08-09 14:52
Yury, I have no objections. Furthermore, it would be nice to expose the coroutine object publicly, like curio and trio do. It would make life simpler for me in some cases.
msg323329 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-08-09 14:55
> I also couldn't figure out yet why PyUnicode_Check() was necessary in the first place. Doesn't PyObject_Str() just increment the refcount if the argument is already a string?

`str()` returns its argument if it's exactly a `builtins.str` instance.  If it's a subclass of str, it will construct a `builtins.str` out of it.

>>> class mystr(str):
...     pass
>>> a = mystr('aaa')
>>> str(a) is a
False

So Eric is right, there's a small discrepancy between Python and C version.
msg323330 - (view) Author: Alex Grönholm (alex.gronholm) * Date: 2018-08-09 15:00
Ok, I understand. But is the conversion a bad thing then?
msg323334 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-08-09 15:16
> Ok, I understand. But is the conversion a bad thing then?

It's not a bad thing, it's just that we don't do it in C Task and we do it in pure Python Task.  Eric wants us to synchronize them so that in a very unlikely scenario where someone uses subclasses of str for names they will have exact same behaviour under both Tasks implementations.

I'd say let's just fix the C version to use PyUnicode_CheckExact.  Even though it's highly unlikely somebody ever hits this, there's no reason to keep Python and C implementations even slightly out of sync w.r.t. behaviour.
msg323337 - (view) Author: Alex Grönholm (alex.gronholm) * Date: 2018-08-09 15:26
> It's not a bad thing, it's just that we don't do it in C Task and we do it in pure Python Task.  Eric wants us to synchronize them so that in a very unlikely scenario where someone uses subclasses of str for names they will have exact same behaviour under both Tasks implementations.

Should a new issue be created for this so I can make a PR against it?
msg323338 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-08-09 15:34
Let's just reuse this issue, it's just a small fix.
msg323339 - (view) Author: Alex Grönholm (alex.gronholm) * Date: 2018-08-09 15:36
Which way do we want to change this? Do we want to convert to pure strings or retain the original object? In the latter case both the C and Python implementations (including set_name()) have to be changed.
msg323340 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-08-09 15:39
I'm not too invested in any changes happening at this point, actually. :)  Mostly I happened to be reading through the commit and noticed the inconsistency.  If I had reviewed the PR then I would have asked that it be fixed.  So I figured I'd mention it.

FWIW, I don't expect it would cause any problems.  It could result in a different (between the two implementations) Task repr if the name's type (a str subclass) implements __repr__.  There's also the possibility of side-effects (from the implementation of the name's type).  Neither is a big deal (especially the latter since it's *not* a common use case).  On the other had, the matter is made moot by using PyUnicode_CheckExact(). :)
msg323341 - (view) Author: Alex Grönholm (alex.gronholm) * Date: 2018-08-09 15:44
> On the other had, the matter is made moot by using PyUnicode_CheckExact()

Then, in order to keep the pure Python implementation in sync, we'd have to change it to something like this:

if name is None:
   self._name = f'Task-{_task_name_counter()}'
elif isinstance(name, str):
   self._name = name
else:
   self._name = str(name)

I don't know about you, but it looks pretty awkward to me.
msg323342 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-08-09 16:00
Please just change PyUnicode_Check to PyUnicode_CheckExact in C Task.__init__ and use the same if check in C Task.set_name.
msg323343 - (view) Author: Alex Grönholm (alex.gronholm) * Date: 2018-08-09 16:06
> Please just change PyUnicode_Check to PyUnicode_CheckExact in C Task.__init__ and use the same if check in C Task.set_name.

I'll do that if you say so, but I'm just saying that the C and Python implementations will still remain different in semantics then.
msg323344 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-08-09 16:09
> I'll do that if you say so, but I'm just saying that the C and Python implementations will still remain different in semantics then.

Probably I'm missing something here. How would they be different?
msg323345 - (view) Author: Alex Grönholm (alex.gronholm) * Date: 2018-08-09 16:10
> 
I'll do that if you say so, but I'm just saying that the C and Python implementations will still remain different in semantics then.

Never mind, that was a brain fart. I keep ignoring the "!" in my mind.
msg323497 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2018-08-14 04:32
New changeset aa4e4a40db531f7095513a4b0aa6510f18162a07 by Benjamin Peterson in branch 'master':
Make regular expressions in test_tasks.py raw strings. (GH-8759)
https://github.com/python/cpython/commit/aa4e4a40db531f7095513a4b0aa6510f18162a07
History
Date User Action Args
2018-08-14 04:32:34benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg323497
2018-08-14 04:15:47benjamin.petersonsetpull_requests: + pull_request8235
2018-08-09 20:49:59yselivanovsetresolution: fixed
2018-08-09 16:19:30alex.gronholmsetpull_requests: + pull_request8202
2018-08-09 16:10:11alex.gronholmsetmessages: + msg323345
2018-08-09 16:09:39yselivanovsetmessages: + msg323344
2018-08-09 16:06:05alex.gronholmsetmessages: + msg323343
2018-08-09 16:00:39yselivanovsetmessages: + msg323342
2018-08-09 15:44:29alex.gronholmsetresolution: fixed -> (no value)
messages: + msg323341
2018-08-09 15:39:36eric.snowsetresolution: fixed
messages: + msg323340
2018-08-09 15:36:11alex.gronholmsetmessages: + msg323339
2018-08-09 15:34:27yselivanovsetresolution: fixed -> (no value)
messages: + msg323338
2018-08-09 15:26:42alex.gronholmsetmessages: + msg323337
2018-08-09 15:16:55yselivanovsetmessages: + msg323334
2018-08-09 15:00:20alex.gronholmsetmessages: + msg323330
2018-08-09 14:55:28yselivanovsetmessages: + msg323329
2018-08-09 14:52:23alex.gronholmsetmessages: + msg323328
2018-08-09 14:51:25alex.gronholmsetmessages: + msg323327
2018-08-09 14:47:49yselivanovsetmessages: + msg323326
2018-08-09 14:46:46yselivanovsetmessages: + msg323325
2018-08-09 14:41:24eric.snowsetnosy: + eric.snow
messages: + msg323324
2018-08-08 21:08:30yselivanovsetstatus: open -> closed
resolution: fixed
messages: + msg323303

stage: patch review -> resolved
2018-08-08 21:06:54yselivanovsetmessages: + msg323302
2018-07-29 11:41:30python-devsetkeywords: + patch
stage: patch review
pull_requests: + pull_request8063
2018-07-29 09:25:37alex.gronholmcreate