Skip to content

Memory leak regression upstream in pycrdt>=0.14 requires workaround #270

Description

@dlqqq

TL;DR

Upgrading to @jupyter/ydoc v4 pulls in jupyter-ydoc v4 → pycrdt >= 0.14,
which bundles yrs 0.27. yrs 0.27 changed how observer callbacks are stored
and removed, which introduces two distinct defects:

  1. Memory leak: removing an observer (via unobserve() / dropping the
    Subscription) no longer frees the callback synchronously. Removal is
    deferred and, on an idle document, effectively never happens. Because our
    observers are bound methods of a "room" object, the room — and its whole Doc
    leaks.
  2. Memory-unsafety (segfault): removing a different observer from inside an
    observer callback mutates the observer collection while yrs is iterating it,
    which is undefined behavior. We reproduced a SIGSEGV using only yrs's public
    safe Rust API
    (no Python, no pycrdt).

Both are reproduced below with minimal tests at both the pycrdt (Python) and yrs
(Rust) levels.

Root cause (upstream)

yrs 0.27 reworked observer storage in
y-crdt/y-crdt#619 ("FnMut observer
callbacks"): to allow FnMut callbacks, the previous lock-free observer structure
was replaced with a HashMap<Origin, F>.

Consequences of that change:

  • A HashMap can't be mutated while it's being iterated during event dispatch, so
    removal was made deferred: dropping a Subscription only pushes its key onto
    a pending-removal queue. The queue is drained lazily — only at the start of the
    next subscribe/trigger
    , never at transaction end and never when idle.
  • The immediate keyed-removal path (Observable::unobserve(key)) still calls
    HashMap::remove directly with no re-entrancy guard, so calling it during
    dispatch corrupts the in-progress iteration.
  • Observer dispatch order also became non-deterministic (keys are random
    Origins, so HashMap iteration order varies per run) — a behavior change from
    the pre-0.27 stable ordering. (The #619 change already had to convert an
    ordering-dependent test to a set comparison.)

Why it leaks for us (the reference cycle)

JSD and Jupyter Collaboration share the same model: a "room" owns a Doc and
registers observers to broadcast/persist/forward document changes. Those observers
are methods of the room, so each callback captures self (the room). pycrdt
additionally wraps the callback to capture the Doc. That forms a cycle crossing
the Python/Rust boundary:

observer callback ──(captures)──▶ Room ──▶ Doc
      ▲                                       │ owns
      └────── yrs Observer HashMap ◀──────────┘

Python's GC can't see the reference held inside the Rust HashMap; Rust's
refcounting doesn't collect cycles. The only thing that breaks the cycle is
removing the callback from the HashMap. Pre-0.27, unobserve() did that
synchronously. In 0.27 it's deferred — so on an idle room being torn down (no
further transactions), the callback lingers and the whole Doc leaks.

Two observer kinds

pycrdt exposes two observer flavors; both are affected but differ mechanically:

  • Shared-type observers (Text, Map, Array, Xml*) — stored directly on
    the type's branch; removal is lock-free.
  • Document observers (Doc.observe, Doc.observe_subdocs) — stored inside the
    document Store, behind a write lock. Immediate removal needs that lock,
    which is unavailable mid-dispatch (a transaction already holds it) → attempting
    it returns ExclusiveAcqFailed.

Test matrix (2 observer kinds × 3 scenarios)

Property under test in every cell: after unobserve(), is the callback — and
everything it captures (its bound receiver + the Doc) — actually released?

Verified via weakref (Python) / Arc::strong_count (Rust) after GC. We assert
on collectability, not on "the callback stops firing" (a callback can stop firing
yet stay retained — that's the leak).

Scenarios:

  1. idle_unobserveunobserve() from ordinary code, no transaction in flight
    (e.g. room teardown).
  2. reentrant_unobserve — an observer removes itself from inside its own
    callback (mid-transaction).
  3. reentrant_multi — one observer removes a different, still-registered
    observer from inside its callback (mid-dispatch).

Results on stock pycrdt 0.14.1 (Python)

observer kind idle_unobserve reentrant_unobserve reentrant_multi
shared-type LEAK ok ok* (deferred, no crash)
document LEAK ok ok* (deferred, no crash)

* stock defers removal, so reentrant_multi doesn't crash — it just (like the
idle case) fails to free promptly. Deferral trades correctness for safety.

Results at the pure-yrs 0.27.2 level (Rust, public safe API, no pycrdt)

test result
remove observer outside a transaction FAIL — leak (Arc::strong_count == 2; callback never dropped)
observer removes itself mid-transaction PASS (drained by a later transaction)
observer removes a different observer mid-transaction SIGSEGV (signal 11)

The pure-Rust reproductions are the important ones: they prove the leak and the
segfault are yrs defects, independent of the Python bindings. Test sources:

  • Rust: yrs/tests/observer_gc.rs (against tag v0.27.2)
  • Python: tests/test_observe_gc.py (2×3 matrix)

Candidate workarounds we evaluated (pycrdt-level)

Two approaches were prototyped on branches of a local pycrdt checkout. Neither is
a clean, complete fix; both are recorded for context.

  • Keyed immediate unobserve (branch 20260701-jsd-observer-gc-fix): register
    shared-type observers with an explicit key and remove synchronously via yrs's
    keyed unobserve. Fixes shared/idle_unobserve. Does not fix document
    observers (lock). Converts shared/reentrant_multi from a leak into a
    SEGFAULT
    (immediate mid-iteration removal).
  • Dummy-drain (branch 20260702-dummy-drain-experiment): after each
    unobserve(), register+drop a throwaway observer to force the pending queue to
    drain — the same trick
    pycrdt#393 used in its tests. Fixes
    both idle cases. But it panics (ExclusiveAcqFailed) on document
    reentrant cases (registering an observer needs the store lock held mid-dispatch),
    and still segfaults on shared/reentrant_multi.

Takeaway: every non-stock mitigation either reintroduces the segfault or panics in
the reentrant cases, because the underlying hazards (deferred-only drain; unguarded
mid-iteration removal) live in yrs. A complete fix likely needs yrs to either
drain pending removals at transaction end and/or expose a safe flush API, and to
guard keyed removal against mid-iteration mutation.

What we need to do next

  • Distill this into a focused, neutral pycrdt/y-crdt issue (the segfault
    is a safe-API soundness bug — likely the strongest lead).
  • Decide on a JSD-side interim workaround so blocked teams can proceed
    (see companion effort — testing the dummy-observer drain inside JSD's room
    teardown).
  • Cross-check behavior against yrs 0.26 to document the regression window.

Environment

  • pycrdt 0.14.1, bundling yrs 0.27.2
  • Reproduced on macOS (arm64), CPython 3.10
  • Pulled in transitively by jupyter-ydoc v4 / @jupyter/ydoc v4

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions