Skip to content

fix: full PyPy support — deterministic stream cleanup + assignor livelock fix, all PyPy skips removed#716

Merged
wbarnha merged 8 commits into
masterfrom
claude/pypy-agen-cleanup
Jul 21, 2026
Merged

fix: full PyPy support — deterministic stream cleanup + assignor livelock fix, all PyPy skips removed#716
wbarnha merged 8 commits into
masterfrom
claude/pypy-agen-cleanup

Conversation

@wbarnha

@wbarnha wbarnha commented Jul 20, 2026

Copy link
Copy Markdown
Member

Description

Makes faust run and test green on PyPy, and removes every skipif(PyPy) marker
in the test suite
(21 decorators covering 30 tests, added wholesale against
PyPy 3.9 in #530/#621 and never re-validated on a modern PyPy). Two independent
root causes, both proven on a real PyPy interpreter.


Root cause 1 — GC-timing-dependent stream cleanup (faust/streams.py)

Cleanup for faust's stream async generators lives in finally blocks that only
run when the generator is finalized. When a caller abandons one (break
inside async for), CPython's refcounting finalizes immediately; PyPy defers
finalization to a GC cycle that may never come
(PyPy CPython-differences,
PEP 525, PEP 533).
Affected duties: acking consumed events, restoring enable_acks, detaching
the buffering processor, and channel detach.

Fix (minimal): Stream tracks the async generators it hands out in a
WeakSet and closes leftovers in on_stop():

  1. the take() family — take, take_events, take_with_timestamp, noack_take.
  2. the main iterator from __aiter__ (_py_aiter/_c_aiter) — its inner
    finally acks the last-yielded event. (Service.stop() sets _stopped
    before awaiting on_stop(), so the iterator finally's re-entrant
    self.stop() is a guarded no-op.)

The WeakSet keeps CPython's prompt refcount finalization — and therefore all
existing CPython behavior — fully intact.


Root cause 2 — CopartitionedAssignor standby livelock (faust/assignor/)

_assign_round_robin frees a slot on an exhausted client by evicting an
arbitrary partition (set.pop) and re-queueing it. When that arbitrary
victim is the partition's only possible home, two partitions ping-pong between
the work queue and the same client forever
. Whether it happens depends on set
iteration order, so identical input completes in ~0.3ms on CPython but livelocks
indefinitely on PyPy — the real reason the copartitioned-assignor property tests
were skipped there (not, as first assumed, mere JIT-warm-up slowness).

Minimal reproducer, deterministic even at PYTHONHASHSEED=0:

input CPython PyPy (before) PyPy (after)
4 clients / 16 partitions / 1 replica 0.0003s >25s (livelock) 0.0009s
64 / 256 / 32 0.1s >20s 0.09s

Fix: evict a victim that some other not-yet-exhausted client can
immediately take, in deterministic (sorted) order, so the re-queued partition
makes forward progress instead of bouncing straight back. Terminates on every
interpreter and keeps the assignment valid. Any replicas >= 1 case exercises
the standby path.


Test-suite changes

  • All 21 skipif(PyPy) decorators removed (functional/unit streams, agents,
    meticulous assignor).
  • wait_for_stream_ack rewritten to close the take()/iterator generator
    explicitly (await agen.aclose()) instead of leaning on asyncio.sleep(0)
    yields — runs the acking finally deterministically on every interpreter.
  • tests/conftest.py: event_loop fixture override — on PyPy, run
    gc.collect() + loop.shutdown_asyncgens() before closing each test's loop,
    so deferred finalizers execute inside the owning test instead of landing on a
    dead loop and leaking tasks/warnings into later tests.
  • Regression tests: test_take_cleanup_on_stream_stop and
    test_aiter_cleanup_on_stream_stop hold a strong reference to the
    generator (blocking finalization on every interpreter, mimicking PyPy) and
    assert the stop path performs the cleanup; test_standby_assignment_terminates
    guards the assignor livelock. All verified to fail without their fix.
  • The copartitioned-assignor property tests keep their original
    deadline=4000/full example budget — no PyPy-specific relaxation needed once
    the livelock is fixed (they pass in ~4s on PyPy).
  • CI: PyPy leg timeout-minutes 10 → 15 (now runs the formerly-skipped tests;
    PyPy is slower and the old cap already clipped the run at ~96%).

Verification (real PyPy 3.9, NO_CYTHON=1)

  • tests/functional/test_streams.py — 54 passed.
  • tests/unit — 1674 passed with the strict error::ResourceWarning filter,
    zero regressions from the event_loop override.
  • tests/meticulous/assignor — 4 passed in ~4s, stable across four
    PYTHONHASHSEED values.
  • CPython: same suites green with Cython enabled and NO_CYTHON=1.
  • flake8/black/isort clean.

🤖 Generated with Claude Code

wbarnha and others added 3 commits July 19, 2026 16:02
The `test_take` / `test_take_wit_timestamp*` tests asserted that an event
consumed via `stream.take()` had been acked after exactly two
`await asyncio.sleep(0)` yields.  `take()` acks consumed events from a
background task, and on slower interpreters (notably PyPy) that task hasn't
run within two event-loop ticks, so `event.message.acked` was still False and
the assertion flaked -- as seen on the PyPy CI leg.

Replace the fixed sleeps with `wait_for_stream_ack()`, which polls for the ack
(up to a 1s timeout) instead of assuming a fixed number of ticks.  The existing
assertions are unchanged, so a genuinely missing ack still fails the test with
the same signal after the timeout.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HHPL4VFWQRQPpjR1gXSKyL
…stop

The cleanup for Stream.take() / take_events() / take_with_timestamp() /
noack_take() -- acking consumed events, restoring enable_acks, and
detaching the buffering processor -- lives in `finally` blocks that only
run when the generator is finalized.  When a caller abandons the
generator (`break` inside `async for`), CPython's reference counting
finalizes it immediately, but PyPy defers finalization to a future GC
cycle that may never come: acks are withheld indefinitely, the stream is
left with acks disabled, and the buffering processor stays attached.
(This is the root cause of the take() test failures on the PyPy CI leg,
demonstrated on a real PyPy: the generator's finally never runs without
an explicit gc.collect().)

Track every buffering generator in a WeakSet and close leftovers
explicitly in Stream.on_stop(), making cleanup deterministic on any
interpreter.  The WeakSet keeps CPython's prompt refcount finalization
(and therefore existing behavior) fully intact; already-exhausted
generators make aclose() a no-op, and a generator still running in
another task is skipped (its owner cleans up).

The new test holds a strong reference to the generator -- blocking
finalization on every interpreter, mimicking PyPy -- and asserts the
stream's stop path performs the cleanup; it fails without this fix on
CPython as well.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HHPL4VFWQRQPpjR1gXSKyL
@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.18%. Comparing base (0a92897) to head (4ab51a5).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #716      +/-   ##
==========================================
+ Coverage   94.15%   94.18%   +0.02%     
==========================================
  Files         104      104              
  Lines       11136    11154      +18     
  Branches     1201     1202       +1     
==========================================
+ Hits        10485    10505      +20     
+ Misses        550      548       -2     
  Partials      101      101              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

wbarnha and others added 2 commits July 20, 2026 13:33
Extend the on_stop generator-cleanup mechanism to the main stream
iterator (__aiter__ -> _py_aiter/_c_aiter), whose inner finally acks the
last-yielded event and whose outer finally detaches from the channel --
previously only the take() family was tracked, so a plain
`async for value in stream: break` still leaked its cleanup to GC timing
on PyPy.  Service.stop() sets _stopped before awaiting on_stop(), so the
outer finally's re-entrant self.stop() is a guarded no-op.

Make the test suite PyPy-clean and remove every skipif(PyPy) marker
(21 decorators covering 30 tests, added wholesale against PyPy 3.9 in
#530/#621 and never re-validated):

* tests/conftest.py: override the event_loop fixture to run gc.collect()
  + loop.shutdown_asyncgens() before closing the loop on PyPy, so
  deferred async-generator finalizers run inside the owning test instead
  of landing on a dead loop and leaking tasks/warnings into later tests.
* tests/meticulous: disable the 4s hypothesis deadline on PyPy -- JIT
  warm-up makes the first examples orders of magnitude slower than
  steady state, tripping DeadlineExceeded on correct code.
* New regression test test_aiter_cleanup_on_stream_stop mirrors the
  take() one: holds a strong reference (blocking finalization on every
  interpreter, mimicking PyPy) and asserts stop-time cleanup acks the
  event.  Verified to fail without the __aiter__ tracking.

Verified on CPython: functional+unit+meticulous suites green with Cython
enabled AND with NO_CYTHON=1 (the pure-Python paths PyPy uses), and
green under a PyPy-semantics simulation (an asyncgen firstiter hook
holding strong references to defeat refcount finalization).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HHPL4VFWQRQPpjR1gXSKyL
@wbarnha wbarnha changed the title fix(streams): close abandoned take() generators deterministically on stop (PyPy correctness) fix(streams): deterministic async-generator cleanup — full PyPy support, all PyPy skips removed Jul 20, 2026
wbarnha and others added 2 commits July 20, 2026 15:49
CopartitionedAssignor._assign_round_robin frees a slot on an exhausted
client by evicting an arbitrary partition (set.pop) and re-queueing it.
When that arbitrary victim is the partition's only possible home, two
partitions ping-pong between the work queue and the same client forever.
Whether it happens depends on set iteration order, so identical input
(e.g. 4 clients / 16 partitions / 1 replica) completes in <1ms on CPython
but livelocks indefinitely on PyPy -- the reason the copartitioned-assignor
property tests were skipped there.

Evict a victim that some other not-yet-exhausted client can immediately
take, in deterministic (sorted) order, so the re-queued partition makes
forward progress instead of bouncing straight back. This terminates on
every interpreter and keeps the assignment valid.

Unskip the three property tests on PyPy (they now pass with the full
example budget in ~4s) and add a regression test asserting the standby
path terminates for any replicas >= 1.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HHPL4VFWQRQPpjR1gXSKyL
…I timeout

Replace the timeout-poll wait_for_stream_ack helper with an explicit
generator aclose(): the take()/iterator finally blocks (which ack consumed
events) only run when the generator is finalized, and abandoning it defers
that to interpreter finalization -- immediate on CPython via refcounting,
deferred to a maybe-never GC cycle on PyPy. Closing it runs the finally
now, deterministically, on every interpreter. Callers hold the generator
and close it instead of leaning on asyncio.sleep(0) yields.

The unit aiter test drains via a bounded gc.collect() + sleep loop for the
same reason (its agent-actor generator can't be reached to close directly).

Bump the PyPy CI leg's timeout from 10 to 15 minutes: it now runs the
formerly-skipped tests and PyPy is slower, and the old cap already clipped
the run at ~96%.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HHPL4VFWQRQPpjR1gXSKyL
@wbarnha wbarnha changed the title fix(streams): deterministic async-generator cleanup — full PyPy support, all PyPy skips removed fix: full PyPy support — deterministic stream cleanup + assignor livelock fix, all PyPy skips removed Jul 20, 2026
…r on stop

on_stop() closes the async generators the stream handed out, skipping any
that are still executing in another task (an agent actor iterating the
stream during a rebalance).  That "still running" case raises RuntimeError
on CPython but ValueError ("async generator already executing") on PyPy, and
only RuntimeError was caught -- so on PyPy the ValueError propagated out of
on_stop, aborting the stream's stop mid-way and leaving stale entries in the
topic's active_partitions.  That tripped the conductor's
`active_partitions.issubset(assigned)` assertion during isolated-partition
rebalances (test_agent_isolated_partitions_rebalancing).

Catch both exception types in on_stop and in the wait_for_stream_ack test
helper, so closing a running generator is always the intended no-op.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HHPL4VFWQRQPpjR1gXSKyL
@wbarnha
wbarnha merged commit f77a678 into master Jul 21, 2026
24 checks passed
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.

1 participant