Repository navigation
Fix: after a reopen, is_alive() was False while reading and join() didn't wait - #4
Merged
Merged
Conversation
#3 ran the first open() in self and later ones in fresh threads, so after a reopen reader.is_alive() was False while the reader was reading and reader.join() returned at once. Every open() now gets its own worker thread and is_alive/join follow it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Post-merge refute review of #3.
Defect: #3 runs the first
open()inself(the reader is aThread) and every later one in a freshthreading.Thread. After a reopen, the inheritedreader.is_alive()describes the finished first-run thread, so it isFalsewhile the reader is busy producing.reader.join()then returns immediately instead of waiting for the worker. The existing testtest_stop_does_not_shadow_thread_internalsshowsclose(); join(); assert not is_alive()is the intended usage, and that pattern silently stops meaning anything after the first cycle. #3's docstring claimed the Thread API kept behaving as before.Fix: every
open()starts its own worker thread, andis_alive()/join()follow the current worker. A reader started the legacy way, withstart()instead ofopen(), has no worker and falls back toThread's own. Running the first open in a fresh thread too, rather than inself, is what keeps #3'stest_reopen_does_not_leak_worker_threadsvalid unchanged:_workerobjects are distinct threads with accurate liveness.Test:
test_thread_api_follows_the_current_worker_after_reopen. It fails on master and passes here. All 6 tests pass (3 runs, py3.10).Self-reviewed only (the dispatching run disallowed sub-agents).
🤖 Generated with Claude Code