Skip to content

Simple chatbot: consider removing cleanup lock from sequential lifecycle #3655

Open

Description

@JEFFTIMES

While studying the simple chatbot's context-management lifecycle, I found Server._cleanup_lock confusing because the example does not appear to invoke cleanup concurrently on the same Server.

Source: examples/clients/simple-chatbot/mcp_simple_chatbot/main.py.

The current call paths are sequential:

  • Server.initialize() awaits self.cleanup() on initialization failure before re-raising.
  • ChatSession.start() awaits cleanup_servers() in its initialization-error handler and again in its finally block.
  • cleanup_servers() awaits each server's cleanup in reverse server order.

Repeated cleanup calls therefore do not overlap in the demonstrated flow. A single AsyncExitStack.aclose() already awaits the registered exits sequentially in reverse order, so the lock is not needed to make session teardown precede transport teardown.

Suggested simplification: remove the _cleanup_lock initialization and the async with self._cleanup_lock: wrapper, retaining the existing cleanup body and error handling. Alternatively, a brief comment explaining an intended concurrent-caller use case would clarify why the lock is retained.

This is an example-code clarity/simplification request based on source inspection, not an observed runtime bug or a claim that locks are unnecessary for concurrent cleanup generally. Removing it would remove serialization for callers that use this class concurrently outside the demonstrated flow; that tradeoff needs maintainer judgment. No runtime tests were performed for this report.

Reporting first per the contribution guide. AI assistance: prepared with OpenAI Codex after discussing the example and inspecting its cleanup call paths.

Activity

  1. added
    enhancementRequest for a new feature that's not currently supported
    on Oct 8, 2026
  2. Ben-Home commented on Oct 8, 2026

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementRequest for a new feature that's not currently supported

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions