Skip to content

fix(tests): use spawn instead of fork for lock-test subprocesses - #1431

Merged
igorls merged 1 commit into
developfrom
fix/test-multiprocessing-spawn
May 9, 2026
Merged

fix(tests): use spawn instead of fork for lock-test subprocesses#1431
igorls merged 1 commit into
developfrom
fix/test-multiprocessing-spawn

Conversation

@igorls

@igorls igorls commented May 9, 2026

Copy link
Copy Markdown
Member

Summary

Recent CI runs have been hanging on Linux 3.13 and macOS while Linux 3.9 / 3.11 and Windows finish normally. Same hang signature on every PR (#1396, #1430): pytest step starts, jobs run for 33+ minutes with no progress, get cancelled.

The cause is in two test files that explicitly request the fork start method for multiprocessing:

# tests/test_palace_locks.py and tests/test_chroma_collection_lock.py
start_method = "spawn" if os.name == "nt" else "fork"
return multiprocessing.get_context(start_method)

By the time these tests run, the pytest parent is multi-threaded — chromadb and onnxruntime both spawn background threads on import. When fork snapshots that state into a child without the threads themselves, any lock held by another thread at fork time stays locked in the child forever. Python 3.13 explicitly warns about this (we saw the DeprecationWarning 10 times in a local 3.13 run), and the timing window where Python's own internal threads hold locks widened in 3.13 enough to make the deadlock reproducible.

macOS hits a parallel issue: Apple's CoreFoundation forbids fork-without-exec. Anything ONNX/Obj-C-bridge-touching that loaded in the parent will silently hang the moment the forked child touches the same library — independent of Python version.

Change

Switch both _get_mp_context() helpers to use multiprocessing.get_context("spawn") unconditionally. Lock-file semantics are unchanged: spawn inherits os.environ (including monkeypatched HOME), which is all these tests need from the parent. Trade-off is per-Process import overhead (~0.5s on Linux), bounded by the small number of subprocesses these tests fork (10 across both files).

Test plan

  • uv run --python 3.13 pytest tests/test_chroma_collection_lock.py tests/test_palace_locks.py -v → 14 passed in 6.58s
  • ruff check and ruff format --check (CI-pinned ruff>=0.4.0,<0.5) → clean
  • CI green on Linux 3.9/3.11/3.13, Windows, macOS, lint
  • Linux 3.13 + macOS no longer hang

Notes

test_palace_locks.py and test_chroma_collection_lock.py spawned child
processes with the ``fork`` start method on POSIX. Under Python 3.13
this deadlocks reliably enough to hang the Linux 3.13 and macOS CI jobs
indefinitely while Linux 3.9 / 3.11 / Windows complete normally.

Root cause: by the time these tests run, the pytest parent process is
multi-threaded — chromadb and onnxruntime both spawn background threads
on import. ``fork`` snapshots the parent's address space into the
child without those threads, so any lock another thread held at fork
time stays locked in the child forever. Python 3.13 widened the window
where Python's own internal threads can be holding locks (hence the new
DeprecationWarning that fired ten times in our local 3.13 run).

macOS hits a related but distinct issue: Apple's CoreFoundation
explicitly forbids fork-without-exec; once anything in the parent has
loaded a CF-using library (ONNX, anything via Objective-C bridges) a
forked child will silently hang the moment it touches the same
library.

Switching to ``spawn`` re-imports modules in the child (~0.5s overhead
per Process — measurable but bounded), which is the standard fix for
both classes of bug. Lock-file semantics are unchanged: ``spawn``
inherits ``os.environ`` (including monkeypatched ``HOME``), which is
all these tests need from the parent.

Locally on Python 3.13: all 14 lock tests pass in 6.58s.
@igorls
igorls requested a review from milla-jovovich as a code owner May 9, 2026 23:11
Copilot AI review requested due to automatic review settings May 9, 2026 23:11

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the multiprocessing start method to always use "spawn" in tests/test_chroma_collection_lock.py and tests/test_palace_locks.py. This change addresses potential deadlocks in multi-threaded environments under Python 3.13 and ensures compatibility with macOS, where fork-without-exec is restricted. I have no feedback to provide.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses CI hangs caused by using the fork multiprocessing start method in lock-related tests after the pytest parent process has become multi-threaded (notably due to chromadb/onnxruntime imports). It standardizes these tests to use spawn across all platforms to avoid fork-related deadlocks and macOS fork restrictions.

Changes:

  • Switch _get_mp_context() in two test modules to always return multiprocessing.get_context("spawn").
  • Update docstrings to document why fork is unsafe in this test environment (Python 3.13 thread-at-fork issues, macOS CoreFoundation behavior).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/test_palace_locks.py Use spawn unconditionally for subprocess lock contention tests; expand rationale in docstring.
tests/test_chroma_collection_lock.py Use spawn unconditionally for subprocess lock contention tests; expand rationale in docstring.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@igorls
igorls merged commit 7d5e6c4 into develop May 9, 2026
10 checks passed
@igorls
igorls deleted the fix/test-multiprocessing-spawn branch May 9, 2026 23:25
This was referenced May 10, 2026
arnoldwender pushed a commit to arnoldwender/mempalace that referenced this pull request May 10, 2026
Bumps version 3.3.4 → 3.3.5 across pyproject.toml, version.py, plugin
manifests, README badge, and uv.lock. Flips CHANGELOG.md from
``[3.3.5] — unreleased`` to ``[3.3.5] — 2026-05-09`` and adds entries
for the four PRs that landed after the bug-fix block was authored:

- Bug Fixes: MemPalace#1396 (tool_search retry on transient HNSW flush)
- Documentation: MemPalace#1385 (CONTRIBUTING git-identity guidance, closes MemPalace#1317)
- Internal: MemPalace#1431 (test multiprocessing fork → spawn)
- Internal: MemPalace#1430 (test sqlite connection lifecycle via contextlib.closing)

The four open issues remaining on the v3.3.5 milestone (MemPalace#1266, MemPalace#1253,
MemPalace#1092, MemPalace#1082) have been moved to v3.4 — they form the concurrent-writer
/ HNSW corruption cluster that needs deeper work than this cycle could
absorb.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants