agronholm / anyio

High level asynchronous concurrency and networking framework that works on top of either trio or asyncio
MIT License
1.79k stars 137 forks source link

in py3.8+ the first asyncio.Task object is allocated with the same ID every time #324

Open graingert opened 3 years ago

graingert commented 3 years ago

This is a problem in anyio because TaskInfo's __eq__ goes on to compare dead object IDs

>>> async def current_task():
...     return anyio.get_current_task()
... 
>>> t = anyio.run(current_task)
>>> u = anyio.run(current_task)
>>> t == u
True
>>> t.id == u.id
True
graingert commented 3 years ago

@agronholm I think this is a bug in Trio too - it's just convenient that trio tasks happen not to be allocated in the same location

graingert commented 3 years ago

I think anyio should maintain its own counter of tasks and use a WeakKeyDictionary to lookup tasks in and return that count

eg: https://github.com/python-trio/trio/blob/6754c74eacfad9cc5c92d5c24727a2f3b620624e/trio/_core/_run.py#L1071

agronholm commented 3 years ago

But since AnyIO is not responsible for creating all tasks, how would that counter work?

graingert commented 3 years ago

Anyio would need to maintain a WeakKeyDictionary from the concrete task object to a TaskInfo

agronholm commented 3 years ago

I don't understand how that relates to counters. What are we counting then?

graingert commented 3 years ago

number of TaskInfo objects created, eg:

@dataclass
class TaskInfo:
   ...
   id = field(default_factory=itertools.count().__next__, init=False)
agronholm commented 3 years ago

Ok, but that means that IDs would not necessarily be in the same sequence that the underlying Tasks are created. Are we okay with that?

graingert commented 3 years ago

well they're not currently in sequence, they're just the whim of the allocator

agronholm commented 3 years ago

Alright. Then, finally, could there be a situation where an underlying Task could be assigned two artificial IDs through two TaskInfo objects, perhaps when the first TaskInfo has been destroyed? Would that even be a problem?

graingert commented 3 years ago

anyio task ids would be in sequence if TaskInfo objects are created eagerly when the _task_state entry is created.

graingert commented 3 years ago

Alright. Then, finally, could there be a situation where an underlying Task could be assigned two artificial IDs through two TaskInfo objects, perhaps when the first TaskInfo has been destroyed? Would that even be a problem?

Wouldn't the TaskInfo live as long as the Task?

agronholm commented 3 years ago

Wouldn't the TaskInfo live as long as the Task?

They currently don't – they are throwaway objects which only live as long the recipient carries them in scope.

graingert commented 3 years ago

Anyio would need to maintain a WeakKeyDictionary from the concrete task object to a TaskInfo

agronholm commented 3 years ago

By task do you mean asyncio.Task, or our own _TaskState objects? At any rate, how would this work for counting things?

graingert commented 3 years ago

something like this?

_task_infos = WeakKeyDictionary[object, TaskInfo]

_counter = itertools.count()

class TaskInfo:
    def __init__(self, task):
        self.id = next(_counter)
        ti = get_async_lib().extract_task_info(task)
        self.parent_id = ti.parent_id
        self.coro = ti.coro

def get_current_task_token():
    library = sniffio.current_async_library()
    if library == "trio":
        return trio.lowlevel.current_task()
    elif library == "asyncio":
        return asyncio.current_task()
    else:
        raise RuntimeError("unsupported")

def get_current_task():
    task = get_current_task_token()
    try:
        return _task_infos[task]
    except KeyError:
        _task_infos[task] = task_info = TaskInfo(task)
       return task_info
agronholm commented 3 years ago

Right, if we keep the TaskInfo objects around, then I can see a counter system working. I was just hoping we wouldn't have to.