Repository navigation
Conversation
A forked child inherits codec_pipeline._pool pointing at threads that do not exist in the child, plus the parent's idle-semaphore permits. Submissions up to the permit count are consumed without spawning a thread, so small FusedCodecPipeline reads hang forever. Add an after_in_child hook that drops the inherited pool, size, and lock. The lock is recreated because it may have been held by a thread that no longer exists in the child. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Move the zarr-developers#4478 regression test next to the other FusedCodecPipeline pool tests and pin `codec_pipeline.max_workers` to 4 through the shared `_FUSED_POOL_CONFIG`. With the default, a 1-CPU host resolves `max_workers` to 1, never creates the pool, and passes the test without exercising the bug. The test now asserts that the parent created the pool before forking, and it runs on macOS, where the fork start method is available. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4504 +/- ##
=======================================
Coverage 94.71% 94.71%
=======================================
Files 94 94
Lines 13663 13663
=======================================
Hits 12941 12941
Misses 722 722 🚀 New features to boost your workflow:
|
d-v-b
marked this pull request as ready for review
October 9, 2026 17:41
This branch has not been deployed
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.
Little follow-up to #4478 that hardens the tests
🤖 AI text below 🤖
This hardens the regression test for #4478 added in #4483. It pins
codec_pipeline.max_workersso the test exercises the bug on any host, and moves the test next to the otherFusedCodecPipelinepool tests._FUSED_POOL_CONFIG(max_workers=4). With the default,max_workersresolves toos.cpu_count(); on a 1-CPU host it is 1, the parent never creates the pool, and the test passes whether or not the fix is present. The test also asserts that the parent created the pool before forking.tests/test_array.pytotests/test_fused_pipeline.py, after the other pool tests.The agent checked that the test passes with the #4483 fix and fails with a 30 s
TimeoutErrorwhen thesrc/change is reverted. It did this on macOS (Python 3.12; 20 of 20 passing runs with the fix) and on Linux (Python 3.12 and 3.14). This is a test-only change, so there is no changelog fragment.Stack notes
This branch is built on the head of #4483 (
8d225dac2), so until #4483 merges the diff here includes its changes. After #4483 is squash-merged, the agent will rebase this branch ontomainso that only the committest(codec_pipeline): pin pool size in the fork regression testremains.🤖 Generated with Claude Code