Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
graingert opened this issue Jun 23, 2021 · 16 comments
Labels
asyncio Involves the asyncio backend bug Something isn't working

Comments

@graingert
Copy link
Collaborator

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
@agronholm agronholm added asyncio Involves the asyncio backend bug Something isn't working labels Jun 24, 2021
graingert added a commit to graingert/starlette that referenced this issue Jun 24, 2021
@graingert
Copy link
Collaborator Author

graingert commented Jun 24, 2021

@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 added a commit to graingert/starlette that referenced this issue Jun 28, 2021
graingert added a commit to encode/starlette that referenced this issue Jul 3, 2021
)

* ensure TestClient requests run in the same EventLoop as lifespan

* for lifespan task verification, use native task identity rather than anyio.abc.TaskInfo equality

agronholm/anyio#324

* remove redundant pragma: no cover

* it's now a loop_id not a threading.ident

* replace Protocol with plain Callable TypeAlias

* use lifespan_context to actually open a task group

trio should complain if used incorrectly here.

* assign self.portal once, schedule reset immediately after assignment

* inline apps into their tests

* make task/loop trackers nonlocals
@graingert
Copy link
Collaborator Author

graingert commented Jul 26, 2021

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
Copy link
Owner

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

@graingert
Copy link
Collaborator Author

graingert commented Jul 26, 2021

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

@agronholm
Copy link
Owner

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

@graingert
Copy link
Collaborator Author

graingert commented Jul 26, 2021

number of TaskInfo objects created, eg:

@dataclass
class TaskInfo:
   ...
   id = field(default_factory=itertools.count().__next__, init=False)

@agronholm
Copy link
Owner

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
Copy link
Collaborator Author

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

@agronholm
Copy link
Owner

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
Copy link
Collaborator Author

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

@graingert
Copy link
Collaborator Author

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
Copy link
Owner

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
Copy link
Collaborator Author

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

@agronholm
Copy link
Owner

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

@graingert
Copy link
Collaborator Author

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
Copy link
Owner

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.

hidraco added a commit to hidraco/starlette that referenced this issue Apr 21, 2022
…213)

* ensure TestClient requests run in the same EventLoop as lifespan

* for lifespan task verification, use native task identity rather than anyio.abc.TaskInfo equality

agronholm/anyio#324

* remove redundant pragma: no cover

* it's now a loop_id not a threading.ident

* replace Protocol with plain Callable TypeAlias

* use lifespan_context to actually open a task group

trio should complain if used incorrectly here.

* assign self.portal once, schedule reset immediately after assignment

* inline apps into their tests

* make task/loop trackers nonlocals
dudleyhunt86 added a commit to dudleyhunt86/starlette-develop-with-python that referenced this issue Oct 7, 2022
…213)

* ensure TestClient requests run in the same EventLoop as lifespan

* for lifespan task verification, use native task identity rather than anyio.abc.TaskInfo equality

agronholm/anyio#324

* remove redundant pragma: no cover

* it's now a loop_id not a threading.ident

* replace Protocol with plain Callable TypeAlias

* use lifespan_context to actually open a task group

trio should complain if used incorrectly here.

* assign self.portal once, schedule reset immediately after assignment

* inline apps into their tests

* make task/loop trackers nonlocals
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
asyncio Involves the asyncio backend bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants