Skip to content

Python: scope the todo mutation lock to the store and session - #9870

Closed
Manohar Paturi (ManoharPaturi) wants to merge 1 commit into
microsoft:mainfrom
ManoharPaturi:fix/todo-mutation-lock-scope
Closed

Manohar Paturi (ManoharPaturi) wants to merge 1 commit into
microsoft:mainfrom
ManoharPaturi:fix/todo-mutation-lock-scope

Conversation

@ManoharPaturi

Copy link
Copy Markdown
Contributor

Motivation & Context

TodoProvider serialized its read-modify-write todo operations with a lock that belonged to the provider instance. Todo stores are deliberately shareable, so two providers over one store serialized against nothing: both could load the same state before either saved, reuse the same item id, and the last save silently dropped the other write while both tool calls reported success. The lock was also keyed by session only, so a session reused across asyncio.run boundaries could raise from the loop bound lock, the exact hazard the file access layer documents and guards against.

Description & Review Guide

  • What are the major changes? The lock registry moved from the provider to a module level map keyed by session, then by store identity and event loop, so every provider over one store and session shares one lock, mirroring _store_write_lock in the file access layer.
  • What is the impact of these changes? Concurrent todo mutations from different agents sharing a store now serialize exactly like mutations from one agent, and per loop keys keep a cross asyncio.run session working. The outer map stays weak, so locks for garbage collected sessions are still evicted, and the default in memory store keeps its zero await point fast path.
  • What do you want reviewers to focus on? The regression gate placed after tool capture, because before_run itself loads todo state to inject it into the run, and the eviction test now asserting against the module registry.

Related Issue

Fixes #9868

AI Assistance

  • No material AI assistance was used.
  • This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

The lock lived on the provider instance, so two providers sharing a todo store could both load the same state before either saved, silently dropping one write, and a session reused across event loops could raise from the loop bound lock. The lock registry is now keyed by store, session, and event loop, mirroring the file access write lock.

Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
@ManoharPaturi

Copy link
Copy Markdown
Contributor Author

Understood, and sorry. #9868 now follows the bug report template with the race reproduction, and this PR stays closed until the team agrees with the locking direction.

This branch was previously deployed

1 inactive deployment
github-app-auth — 3ca21511 Deployed Oct 30, 2026 by ManoharPaturi via team_check #8072
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: todo mutations race when two providers share one todo store

2 participants