fix(profiler): Prevent buffer race condition during rapid start/stop cycles#5622
Draft
ericapisani wants to merge 3 commits intomasterfrom
Draft
fix(profiler): Prevent buffer race condition during rapid start/stop cycles#5622ericapisani wants to merge 3 commits intomasterfrom
ericapisani wants to merge 3 commits intomasterfrom
Conversation
…cycles Fix a race condition in ContinuousScheduler.run() where setting self.buffer = None after the sampling loop would destroy buffers created by concurrent ensure_running() calls during rapid stop/start cycles. This caused the profiler to silently drop all samples when the new thread's buffer was destroyed by the old thread's cleanup. The fix uses a local buffer reference for flushing and never sets self.buffer = None from run(). Also updated the profiler_id property to check not self.running first. Co-Authored-By: Claude <noreply@anthropic.com>
Remove the no-op `buffer = None` assignment on a local variable before the function returns. Setting a local variable to None provides no GC benefit since it goes out of scope immediately after. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Contributor
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Bug Fixes 🐛
Documentation 📚
Internal Changes 🔧Openai Agents
Other
🤖 This preview updates automatically when you update the PR. |
Contributor
Codecov Results 📊✅ 13 passed | Total: 13 | Pass Rate: 100% | Execution Time: 8.15s All tests are passing successfully. ❌ Patch coverage is 25.00%. Project has 13866 uncovered lines. Files with missing lines (1)
Generated by Codecov Action |
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
1 participant
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.
Fixes #5621 and PY-2127
Fix a race condition in ContinuousScheduler where the profiler thread's cleanup could destroy a buffer created by a concurrent ensure_running() call.
The Problem
During rapid start/stop cycles, a race condition could occur:
self.buffer = NoneThis race is a latent bug on all Python versions, but manifests as flaky tests on Python 3.6 with gevent due to timing and thread scheduling differences.
The Solution
Two changes prevent the race:
not self.runningcheck to avoid accessing a buffer during shutdownTesting
Added a test that simulates rapid start/stop cycles and verifies run() doesn't null out self.buffer after exiting the sampling loop.