Skip to content

Fix server lifespan overlap teardown#3415

Merged
jlowin merged 2 commits intomainfrom
codex/fix-server-lifespan-manager-race-condition
Mar 7, 2026
Merged

Fix server lifespan overlap teardown#3415
jlowin merged 2 commits intomainfrom
codex/fix-server-lifespan-manager-race-condition

Conversation

@jlowin
Copy link
Member

@jlowin jlowin commented Mar 6, 2026

Motivation

  • Server-level lifespan was tracked by a simple boolean and cleared on any exit, which allowed overlapping in-process client sessions to prematurely tear down shared resources and cause intermittent failures.

Description

  • Add a reference count and lock (_lifespan_ref_count, _lifespan_lock) to FastMCP and make _lifespan_manager increment/decrement the ref count under asyncio.Lock to serialize entry/exit.
  • Ensure the first entrant initializes the user lifespan and provider lifespans, while additional entrants only increment the counter and yield without re-entering teardown logic, and ensure teardown runs only when the final session exits.
  • Add a focused regression test test_server_lifespan_overlapping_sessions that keeps one client alive while another exits and asserts the shared lifespan resource remains open until the last session closes.
  • Files changed: src/fastmcp/server/server.py, src/fastmcp/server/mixins/lifespan.py, tests/server/test_server_lifespan.py.

Testing

  • Ran uv sync successfully to ensure deps were available.
  • Ran the focused lifespan tests with uv run pytest tests/server/test_server_lifespan.py -n auto and they passed (17 passed).
  • Ran the full test suite with uv run pytest -n auto; there were unrelated flaky/timeouts in this environment (11 failed, 1 error) that are not related to the lifespan refcounting change.
  • Ran lint/type checks for modified files with uv run ruff which passed; uv run prek run --all-files failed to initialize hooks due to an external network-retrieval error and is not a code regression.

Codex Task

🤖 Generated with GPT-5.2-Codex
@marvin-context-protocol marvin-context-protocol bot added bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. server Related to FastMCP server implementation or server-side functionality. labels Mar 6, 2026
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a67541378b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +149 to 153
yield
finally:
async with self._lifespan_lock:
self._lifespan_ref_count -= 1
return

Choose a reason for hiding this comment

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

P1 Badge Clear lifespan state when non-owner context is last to exit

When the first-entered session exits before a later overlapping session, teardown runs in the owner branch, but this non-owner branch only decrements _lifespan_ref_count and returns. If that decrement reaches zero, _lifespan_result_set remains True (and _lifespan_result stays stale), so subsequent sessions take the fast path and skip re-entering lifespan entirely even though resources were already torn down; this leaves future requests running against closed/uninitialized lifespan state.

Useful? React with 👍 / 👎.

@jlowin
Copy link
Member Author

jlowin commented Mar 7, 2026

Auto-reviewed: all CI checks green, review comments addressed. Merging.

@jlowin jlowin merged commit 6637bce into main Mar 7, 2026
7 checks passed
@jlowin jlowin deleted the codex/fix-server-lifespan-manager-race-condition branch March 7, 2026 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. codex server Related to FastMCP server implementation or server-side functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant