Skip to content

Cross-process registry writes are unlocked, and per-instance graphs make concurrent writers routine #27

Description

@lisachenko

Noted as out-of-scope in PR #23 (per-instance keying); filing it as its own issue as promised there.

The race

In arena mode, the registry's entries and objects tables are shared HashTables that every process of the family can write: persist() / persistInstance() insert into both, drop() deletes. Nothing serializes those writers — the only locks in PersistentStore are the stripe locks on the mutableHandle() write path. Two processes inserting into one zend_hash concurrently is ordinary memory corruption: bucket slots double-claimed, nNumUsed torn, a table whose iteration order lies.

This was largely theoretical when graphs were class-keyed singletons persisted pre-fork. Per-instance keying (PR #23) made concurrent writers routine-adjacent:

  • a parent persists a task graph at every unpublished spawnParallel() — mid-run, by design;
  • a worker persists a SharedError instance graph at every panic (SharedError::capture()), in whatever process the panic happens;
  • a parent spawning while a worker panics is therefore two simultaneous registry writers with no coordination, in a completely ordinary run of the coroutines runtime.

Why the obvious fix is wrong

A mutex around persist() violates the family's lock discipline, stated in both repos' AGENTS: while holding a native lock, never make an allocating engine call — and persist is little else: it interns strings into the arena, mints tables, clones a whole object graph. An allocation can re-enter the engine under a lock the engine knows nothing about, and a persist-sized critical section would also serialize every spawn against every panic.

The shape a fix needs

Publication, not exclusion — do the allocating work unshared, take a lock only for the pointer-sized commit:

  1. Convert into private staging: the persister builds the graph, snapshots, arena strings and the entry's member/meta tables without touching the shared entries/objects tables (everything it allocates is already arena-resident and address-stable).
  2. Publish under a short critical section: inserting N pre-built pointers into the two shared tables is memcpy-and-pointer-swap work — exactly what the lock discipline permits. A dedicated registry stripe (e.g. stripeFor(registry root address), identical in every process) serializes publishers.
  3. drop()'s unlink takes the same stripe; the share-count upsert in adjustShares() needs auditing for the same window (it is an in-place IS_LONG upsert today — single 8-byte store, but two concurrent ±1 adjusters still lose an update).

The engine-side hazard inside step 2 is the same one assertRegistryRoom already guards (an insert must never make the engine grow a shared table), so publication composes with the existing guard rather than adding a new failure mode.

Acceptance criteria

  • A stress rig — one process persisting instance graphs in a loop while a sibling captures panics in a loop — runs clean under the same round counts the existing soaks use.
  • The publish critical section provably makes no allocating engine call (the persister's staging is complete before the lock).
  • adjustShares() from two processes concurrently never loses an increment (asserted by counting, not assumed).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions