Issue 34270: Add names to asyncio tasks

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

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

Thank you for the contribution!

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. :/

> 2. change the check to PyUnicode_CheckExact()

I'd be OK with this, but why is this important?

As a side note, Alex, what do you think about appending coroutine's name to Task's name if the latter is autogenerated?

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.

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.

> 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.

Ok, I understand. But is the conversion a bad thing then?

> 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.

> 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?

Let's just reuse this issue, it's just a small fix.

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.

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(). :)

> 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.

Please just change PyUnicode_Check to PyUnicode_CheckExact in C Task.__init__ and use the same if check in C Task.set_name.

> 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.

> 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?

> 
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.

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

stage: patch review -> resolved