Skip to content

Commit 5abce1f

Browse files
d-v-bbarlowa124devin-ai-integration[bot]claude
authored
test(codec_pipeline): pin the pool size in the fork regression test (#4504)
* fix: reset the codec-pipeline thread pool after fork 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> * docs: add changelog entry for #4483 Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(codec_pipeline): pin pool size in the fork regression test Move the #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 <[email protected]> --------- Co-authored-by: barlowa124 <[email protected]> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <[email protected]>
1 parent 9d20d44 commit 5abce1f

1 file changed

Lines changed: 45 additions & 0 deletions

File tree

‎tests/test_fused_pipeline.py‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
from __future__ import annotations
44

55
import asyncio
6+
import multiprocessing as mp
7+
import sys
68
from dataclasses import dataclass, field, replace
79
from typing import TYPE_CHECKING, Any
810
from unittest.mock import patch
@@ -37,6 +39,7 @@
3739

3840
if TYPE_CHECKING:
3941
from collections.abc import AsyncIterator, Callable, Iterable
42+
from pathlib import Path
4043

4144
from zarr.abc.store import ByteRequest
4245
from zarr.core.buffer import Buffer, NDBuffer
@@ -543,6 +546,48 @@ def read_row_block(i: int, handle: zarr.Array[Any] = fresh) -> np.ndarray:
543546
np.testing.assert_array_equal(fut.result(), data[i * 4 : (i + 1) * 4, :])
544547

545548

549+
def _read_region(arr: zarr.Array[Any], selection: Any) -> np.ndarray:
550+
return np.asarray(arr[selection])
551+
552+
553+
@pytest.mark.skipif(sys.platform == "win32", reason="fork is not available on Windows")
554+
@pytest.mark.filterwarnings(
555+
r"ignore:This process \(pid=\d+\) is multi-threaded, use of fork\(\):DeprecationWarning"
556+
)
557+
def test_forked_child_does_not_reuse_parent_pool(tmp_path: Path) -> None:
558+
"""A forked child must not reuse the parent's codec-pipeline thread pool.
559+
560+
Regression test for https://github.com/zarr-developers/zarr-python/issues/4478.
561+
Executor threads do not survive `fork()`, but the inherited executor keeps the
562+
idle-semaphore permits its parent's workers released. Each submission in the
563+
child consumes a permit instead of starting a thread, so a read of no more
564+
chunks than there are permits queues work that nothing runs.
565+
566+
`max_workers` is pinned rather than left to `os.cpu_count()`: on a 1-CPU host
567+
the default resolves to 1, the parent never creates the pool, and the test
568+
would pass without exercising the bug.
569+
"""
570+
import zarr.core.codec_pipeline as cp_mod
571+
572+
data = np.arange(100).reshape(10, 10)
573+
with zarr_config.set(_FUSED_POOL_CONFIG):
574+
arr = zarr.create_array(
575+
store=str(tmp_path / "a.zarr"), shape=data.shape, chunks=(5, 5), dtype=data.dtype
576+
)
577+
arr[:] = data
578+
# Each multi-chunk read releases one idle permit per completed task, so after
579+
# two 4-chunk reads the pool holds more permits than the child read submits.
580+
arr[:]
581+
arr[:]
582+
assert cp_mod._pool is not None, "the parent read did not create the pool"
583+
with mp.get_context("fork").Pool(1) as pool:
584+
# `[:5]` spans 2 chunks. Without the after-fork reset, both submissions
585+
# are swallowed by the dead pool and `get` times out.
586+
result = pool.starmap_async(_read_region, [(arr, np.s_[:5])])
587+
(read_back,) = result.get(timeout=30)
588+
np.testing.assert_array_equal(read_back, data[:5])
589+
590+
546591
def test_shared_transform_decode_alternating_specs() -> None:
547592
"""A single ChunkTransform must decode chunks of DIFFERENT specs correctly
548593
when calls alternate, exercising eviction/refill of its single-entry

0 commit comments

Comments
 (0)