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

Extensions execution context can easily overlap between parallel executions. #3571

Open
nrbnlulu opened this issue Jul 14, 2024 · 2 comments · May be fixed by #3640
Open

Extensions execution context can easily overlap between parallel executions. #3571

nrbnlulu opened this issue Jul 14, 2024 · 2 comments · May be fixed by #3640
Labels
bug Something isn't working

Comments

@nrbnlulu
Copy link
Member

nrbnlulu commented Jul 14, 2024

Describe the Bug

RN extensions can be singleton instances meaning that the execution context can change during
lifepsan hook execution (if you have parallel operaitons running) :

async def test_execution_context_distinct_on_parallel_execution():

    class EnsureDistinctExtension(SchemaExtension):
        def __init__(self) -> None:
            ...
        async def on_operation(self) -> AsyncIterator[None]:
            query = self.execution_context.query
            yield
            assert self.execution_context.query == query

    @strawberry.type
    class Query:
        @strawberry.field
        async def foo(self) -> str:
            await asyncio.sleep(0.01)
            return "pong"

        @strawberry.field
        async def bar(self) -> str:
            await asyncio.sleep(0.02)
            return "pong"

    schema = strawberry.Schema(query=Query, extensions=[EnsureDistinctExtension()])
    query_foo = "query foo { foo }"
    query_bar = "query bar { bar }"
    for _ in range(10):
        results = await asyncio.gather(*[schema.execute(query_foo), schema.execute(query_bar)])

    for res in results:
        assert not res.errors

I want to remove the execution_context from the extension instance and provide it as an argument to each hook
what do you guys think?
original discussion on discord

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@nrbnlulu nrbnlulu added the bug Something isn't working label Jul 14, 2024
@nrbnlulu nrbnlulu linked a pull request Sep 23, 2024 that will close this issue
11 tasks
@nburns
Copy link

nburns commented Sep 26, 2024

We've seen this in the wild, with an async schema extensions seem to run multiple times since a recent update.

@patrick91
Copy link
Member

@nburns I think #3640 should fix this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants