fix(agentserver): release Windows replay locks on process exit - #49038
Draft
Shikhar Goel (sgoel2be24-cyber) wants to merge 2 commits into
Draft
Shikhar Goel (sgoel2be24-cyber) wants to merge 2 commits into
Shikhar Goel (sgoel2be24-cyber) wants to merge 2 commits into
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 7 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Thank you for your contribution Shikhar Goel (@sgoel2be24-cyber)! We will review the pull request and get back to you soon. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #48603.
On Windows, an abrupt process exit leaves the replay stream's marker file behind, so replacement processes cannot recover the stream. Replace marker-file ownership with a nonblocking
msvcrt.lockingbyte lock, released by the OS when the descriptor closes or its process dies. POSIX locking is unchanged.The sidecar stays on disk deliberately: existence no longer means ownership, and unlinking it could split contenders across different files. Cleanup closes the lock descriptor once; failed acquisition closes both descriptors and preserves non-contention I/O errors. No dependency or public API changes.
Tests cover real subprocess
os._exit(86)recovery with replay/continued writes, rejection while another process is alive and recovery after killing it, Windows stale-sidecar recovery, and mocked Windows error cleanup on any platform. The mocked tests are not evidence of native Windows execution. Four error-path cases fail on unchanged upstream and pass with the fix.Developed with assistance from OpenAI Codex.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines
Local validation on macOS / Python 3.12:
asyncio_mode=auto. Rerunning their entiretest_sse_writer.pywith the responsespyproject.tomlgave 11 passed, resolving all six invocation failures.git diff --check: clean.Commands from the repository root (with the local core/responses packages and dev dependencies installed):
Native Windows execution is pending: this development machine is macOS. The existing agentserver CI matrix includes Windows Server 2022/Python 3.12; no pipeline changes are included. This PR remains a draft pending native Windows validation and review, per repository agent guidance.
Full-core Pyright also reports an existing
_tracing.py:851TracerProvider.add_span_processorerror with the installed telemetry dependencies. The identical error reproduces on unchanged upstreamae81bbf1c9ed59e6f06c48d7aa0e815d8c2396df; the changed streaming module passes with zero errors/warnings.