Skip to content

test: fix GIL subprocess test correctness - #6211

Merged
rwgk merged 5 commits into
masterfrom
rwgk/gil_scoped_maint/fix-test-correctness
Oct 31, 2026
Merged

rwgk merged 5 commits into
masterfrom
rwgk/gil_scoped_maint/fix-test-correctness

Conversation

@rwgk

@rwgk rwgk commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Fix subprocess deadlock detection so startup/shutdown delays do not count as deadlocks. Propagate worker exceptions, correct the internals-count assertion, and rename the free-threaded skip marker. Add regressions for startup/shutdown and worker failures.

The test changes are narrated below, in commit order.

commit 778656f — test: distinguish GIL deadlocks from subprocess startup and shutdown

The intentional-deadlock fixture is a control for the detector: it deliberately creates a situation that cannot complete while the GIL is enabled. The calling thread holds the GIL, starts another thread that tries to acquire it, and waits for that thread with join(). The worker needs the caller to release the GIL; the caller is waiting for the worker to finish. On Python with the GIL disabled, that dependency does not exist and the fixture can return normally.

The harness runs each target in a separate process so the parent can terminate a stuck child. Previously, the intentional-deadlock cases waited 100 milliseconds and treated a still-running child as proof that the expected deadlock had occurred. However, a child can spend more than 100 milliseconds importing Python and extension modules before it reaches the target at all. This matters with Linux's forkserver and macOS/Windows spawn, particularly when importing the full test extension also imports NumPy. The control could therefore pass without exercising the intentional deadlock. Investigating the skip added in #6203 brought this weakness to light.

The fix introduces two multiprocessing.Event objects, event_started and event_finished. Each is a flag shared between the parent and child. The child sets the first immediately before calling the target and sets the second only after the target returns normally:

def _run_in_process_target(target, args, kwargs, event_started, event_finished):
    event_started.set()
    target(*args, **kwargs)
    event_finished.set()

The parent waits for these flags to distinguish three phases:

  1. Startup: allow up to ten seconds for the child to reach the wrapper. Failure produces a specific startup error.
  2. Execution: start the target's execution allowance only after startup. Ordinary tests retain the ten-second allowance; the intentional-deadlock control uses 100 milliseconds. A child that remains alive without returning its target is reported as a possible deadlock.
  3. Shutdown: once the target has returned, allow up to ten seconds for the process to exit. Slow interpreter shutdown produces a separate error and cannot satisfy the deadlock control.

The intentional-deadlock fixture now has one dedicated test that explicitly expects the detector's DEADLOCK report. Ordinary parametrizations contain only targets expected to complete. The helper no longer recognizes the fixture by function identity or turns its timeout into a successful return. The control is skipped on free-threaded builds, where the fixture does not establish the intended GIL dependency. Existing platform handling is retained, including accepting a Windows/PyPy expected-failure report only when its message identifies a detected deadlock.

Two regression cases run a harmless target with a 100-millisecond execution allowance and inject a 200-millisecond delay either before the wrapper starts or after it returns. Both must pass: the delay belongs to startup or shutdown. Removing the corresponding safeguard made each regression fail during validation. Cleanup also terminates a still-running child and joins every successfully started process, including when an exception interrupts the test.

commit c2a4d2e — test: rename free-threaded skip marker to match its condition

Rename skipif_not_free_threaded to skipif_free_threaded. The existing predicate checks the build configuration, Py_GIL_DISABLED, so the marker skips tests on a free-threaded Python build. The new name says directly when the skip happens. The predicate, skip reason, and marked tests are unchanged.

The old name grew out of an understandable ambiguity between describing a test's requirement and describing its skip condition. In #5699, Henry initially called the marker notfreethreaded, which naturally reads as a requirement: run these tests without free threading. I suggested skipif_not_free_threaded to make its purpose easier to read. Adding skipif_ changed how the name reads, while retaining the wording that described the requirement. That is how the misleading “not” entered the name; the underlying skip condition was already correct.

The follow-up #5701 switched the predicate to the build configuration because test collection could enable the GIL on a free-threaded build. The name carried forward. This commit resolves the wording ambiguity without changing that behavior.

commit 867e7df — test: check the internals count for every GIL operation pattern

The assertion in test_multi_acquire_release_cross_module was missing parentheses around its conditional expected value:

# Before
assert len(internals_ids) == 2 if bits % 8 else 1

# After
assert len(internals_ids) == (2 if bits % 8 else 1)

Python parses the old expression as (len(internals_ids) == 2) if bits % 8 else 1. For the 16 values divisible by eight, the assertion therefore checked the truthiness of the literal 1 and never inspected the internals count. It always passed for those cases.

The parentheses make the conditional select the expected count: two when bits % 8 is nonzero, otherwise one. The existing loop now checks the intended invariant for all 128 GIL operation patterns.

commit d11cf36 — test: propagate exceptions from GIL test worker threads

An exception in a Python worker thread does not automatically become an exception in the thread that started it. A worker could raise AssertionError and print a traceback, while the subprocess's main thread joined the workers and returned normally. The child would then exit successfully, allowing the outer test to pass despite the failed assertion.

Each worker now calls the test through a small wrapper that catches BaseException and places it in a thread-safe queue.SimpleQueue. After joining the workers, the calling thread re-raises the first captured exception. In a subprocess test, that exception reaches the child's main thread, gives the child an unsuccessful exit, and makes the outer test fail.

This uses Python's threading queue within one process. The existing sequential and parallel worker arrangements are preserved, and workers are joined before an exception is re-raised. Two regression cases deliberately raise AssertionError in workers and check propagation in both arrangements; they fail with the old helper.

commit d50935a — test: avoid semaphore finalizers in GIL subprocess tests

The first independent CI run of this PR exposed a Linux/PyPy cleanup problem after the GIL tests had passed. A later kwargs/refcount test forced garbage collection and reported hundreds of unraisable exceptions from semaphore finalizers re-entering Python's multiprocessing resource tracker.

The two multiprocessing.Event objects introduced above each allocate five named semaphores with Linux's forkserver start method. Across the per-case subprocess matrix, that creates roughly 710 semaphores. PyPy can retain them until a later collection, concentrating their cleanup in an unrelated test.

This follow-up replaces the two events with ordered messages on one unidirectional pipe. The child sends "started" immediately before calling the target and "finished" only after it returns normally. The parent retains the separate startup, execution, and shutdown allowances. It keeps its writer endpoint open until child cleanup, ensuring that EOF cannot be mistaken for a phase signal; context managers close both parent endpoints and the child's writer.

A new regression makes multiprocessing.Event unavailable and verifies that a real subprocess still completes. It fails with the previous harness. The existing delayed-start, delayed-exit, deliberate-deadlock, and worker-exception tests continue to exercise their respective behavior.

Validation of the semaphore-free follow-up: clean regular/free-threaded CPython 3.14.4 suites passed (1,454 and 1,438 pytest tests; 31 C++ cases and 1,709 assertions each). The full serial Linux PyPy 3.11/8.0.1 Python suite passed (1,065 tests). Explicit spawn checks and all pre-commit hooks passed.

Independent CI and CIBW validation passed, along with the external checks. The Linux/PyPy job passed all 93 GIL tests and the later refcount test without resource-tracker or unraisable errors. The dependent PR #6212 also passed its fresh CI run with this fix included.

Suggested changelog entry:

  • Fix GIL test subprocess timeout handling and worker-thread exception propagation, avoid semaphore-finalizer failures on PyPy, correct the internals-count assertion, and clarify the free-threaded skip-marker name.

rwgk added 4 commits October 24, 2026 08:11
Wait for explicit child-start and function-return signals so slow imports
or interpreter shutdown cannot masquerade as a blocked test. Replace the
intentional-deadlock special case with a dedicated GIL-only detector
control, and add regressions for delayed startup and shutdown.

Validation: CPython 3.14.4 focused GIL suite, 90 passed on regular Python;
72 passed and 18 skipped on free-threaded Python. Spawn checks and the
all-files pre-commit hooks passed. Both timing regressions fail when their
corresponding safeguards are removed.
Rename skipif_not_free_threaded to skipif_free_threaded so the name states
when pytest skips the test. The predicate and test behavior are unchanged.

Validation: all repository pre-commit hooks passed.
@rwgk
rwgk added this pull request to stack #6213 October 24, 2026 17:05
@rwgk
rwgk force-pushed the rwgk/gil_scoped_maint/fix-test-correctness branch from 5168e23 to d11cf36 Compare October 25, 2026 03:11
Replace the started/finished multiprocessing.Events with ordered messages
on a one-way pipe. Each Event allocates named semaphores under forkserver;
the per-case subprocess matrix can accumulate hundreds before PyPy garbage
collection, triggering reentrant resource_tracker cleanup in later tests.

Preserve separate startup, execution, and shutdown allowances. Keep the
parent writer open until child cleanup so EOF cannot masquerade as a phase
signal, and close all connection endpoints deterministically.

Add a regression that disables multiprocessing.Event and runs a real
subprocess successfully. It fails with the previous harness.

Validation: clean CPython 3.14.4 suites passed (1454 regular, 1438
free-threaded; Catch2 31 cases/1709 assertions each). Linux PyPy 3.11/8.0.1
full serial Python suite passed (1065 passed). Spawn checks and all
pre-commit hooks passed.
@rwgk
rwgk merged commit d37783b into master Oct 31, 2026
87 checks passed
@rwgk
rwgk deleted the rwgk/gil_scoped_maint/fix-test-correctness branch October 31, 2026 18:10
@github-actions github-actions Bot added the needs changelog Possibly needs a changelog entry label Oct 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs changelog Possibly needs a changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant