Skip to content

fix: support persistent lifespans in asgi - #231

Open
dom96 wants to merge 1 commit into
mainfrom
dominik/persistent-lifespan
Open

fix: support persistent lifespans in asgi#231
dom96 wants to merge 1 commit into
mainfrom
dominik/persistent-lifespan

Conversation

@dom96

@dom96 dom96 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Ensures that mutable lifespans are persisted across requests. Adds fastapi and asgi-specific tests.

@dom96
dom96 requested review from hoodmane and ryanking13 August 27, 2026 17:19
Comment on lines +485 to +491
async def lifespan_state():
nonlocal lifespan
if lifespan is None:
# Assign before awaiting so concurrent first requests share startup.
lifespan = create_task(start_application(app))
_, state = await lifespan
return state

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A cancelled first request propagates cancellation into this shared task, leaving lifespan permanently cancelled and causing every later request in the isolate to be cancelled. Shield the shared startup task from an individual request cancellation.

Suggested change
async def lifespan_state():
nonlocal lifespan
if lifespan is None:
# Assign before awaiting so concurrent first requests share startup.
lifespan = create_task(start_application(app))
_, state = await lifespan
return state
async def lifespan_state():
from asyncio import shield
nonlocal lifespan
if lifespan is None:
# Assign before awaiting so concurrent first requests share startup.
lifespan = create_task(start_application(app))
_, state = await shield(lifespan)
return state

@ask-bonk

ask-bonk Bot commented Aug 27, 2026

Copy link
Copy Markdown

I'm Bonk, and I've done a quick review of your PR.

Makes ASGI entrypoint lifespan state persist across requests.

  1. High: Posted 1 actionable inline suggestion.

github run

Comment thread packages/runtime-sdk/src/workers/asgi.py Outdated
Comment thread packages/runtime-sdk/src/workers/asgi.py Outdated
@dom96
dom96 force-pushed the dominik/persistent-lifespan branch 2 times, most recently from 864cb45 to 5b8cb5e Compare August 28, 2026 16:00
Comment thread packages/runtime-sdk/src/workers/asgi.py
Comment thread packages/runtime-sdk/src/workers/asgi.py Outdated

@hoodmane hoodmane left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks reasonable, though I'd set _start_future on the AsgiWorkerEntrypoint instance rather than the class and initialize _start_future to None in AsgiWorkerEntrypoint.__init__(). Could adjust it in a followup if you like though.

@dom96
dom96 force-pushed the dominik/persistent-lifespan branch from 5b8cb5e to 8b7b84c Compare September 1, 2026 16:18
@dom96
dom96 force-pushed the dominik/persistent-lifespan branch from 8b7b84c to afea9e1 Compare September 1, 2026 16:29
@dom96

dom96 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

So we do actually want it set on the class because the instance is re-created on every worker invocation. The lifespan is supposed to stay alive for the duration of the lifetime of the isolate. I added a comment to explain this.

@hoodmane

hoodmane commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

the instance is re-created on every worker invocation

That is really weird, can we migrate away from that?

@hoodmane

hoodmane commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

In JavaScript is the WorkerEntrypoint instantiated on every request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants