fix: resolve a container's parent from the change that created it

[zxch3n]

Fixes #1158. Fixes #1161.

Summary

In a document loaded from a snapshot (or created by fork()), touching the metadata of a tree node that was created under an already deleted parent panicked with InternalError: Parent is not registered (arena.rs) while the state lock was held, so the process aborted (WASM: RuntimeError: unreachable). Affected: is_deleted(), edits, events for an imported edit of that metadata (state creation, get_path, with_ancestors), and undo/revert touching it. get_container / has_container / getContainerById also disagreed with a full-history import (None instead of the deleted container).

It also fixes #1161. After an import rejected by state validation was rolled back, change blocks that the import had parsed kept container indices and value slices past the truncated arena, so a later export, checkout, or import crashed.

Root cause

The arena knew a container's parent only from a parsed change (register_container_and_parent_link) or, in a document loaded from a snapshot, from the container's own state entry (the InnerStore parent resolver). A snapshot's change blocks are parsed lazily, and a full snapshot only has state entries for materialized containers. The metadata of a node created inside an already deleted subtree is never materialized, so neither source knew its parent. Shallow snapshots were not affected: shallow export ensures every container of the retention walk, including the metadata of deleted nodes.

Fix

A normal container's ID is the ID of the op that created it. The op log now installs a creator resolver on the arena (ChangeStore::creator_resolver) that loads the change block holding that op; parsing it registers the parent link through the existing register_container_and_parent_link. The answer comes from the history, the same as for a full-history import, and it also works for snapshots exported by older versions.

• SharedArena::get_parent tries the registered edge, then the state KV, then the creator resolver. SharedArena::get_depth resolves the ancestor chain the same way before taking the arena lock, because parsing registers containers and would deadlock under it; it fails fast on a parent cycle. DocState::does_container_exist (behind get_container, has_container, WASM getContainerById) calls find_created_container for a normal ID that is not registered, after the state check, so a container with a state entry is still answered without parsing.
• Locking. The resolver is the only access to the change store that does not hold the document's op log lock (it runs under the state lock, or under no document lock while events are emitted). ChangeStore now takes its locks in one documented order: root_history_names, external_kv, inner, external_vv, then the arena's. get_parsed_block_in, flush_and_compact, and get_change_by_lamport_lte used to take inner before external_kv. That was harmless while the op log lock serialized every caller, but with the resolver, is_deleted() on one thread and len_changes() / export / vv_to_frontiers on another deadlocked (found in review). The resolver looks for a cached block under inner alone, then takes external_kv and inner in order and looks again before it loads the block. It holds weak references, so there is no Arc cycle; SharedArena::fork drops it.
• When nothing knows the parent, get_parent returns None instead of panicking, but only after the history was asked. The resolver answers Loaded (a block holds the op, now parsed) or Absent (none does: an ID that is not a container, an op not received yet, or a shallow document whose creating op was trimmed and whose state does not hold the container). Shallow export keeps everything that can be alive or revived, so such a container is dead at the root, and no path leads to it. Callers treat it like a deleted container (is_deleted → true, not cached; get_path / get_depth → None). is_deleted() aborts with "Parent is not registered" on deleted-node metadata in a snapshot-loaded doc #1158's metadata always resolves through its creating op, so this is not an "unresolvable means deleted" stopgap. Broken invariants still fail fast:
  • the resolver panics when a block holds the op but cannot be parsed;
  • an arena without a resolver panics;
  • ContainerWrapper::new panics for a container without a parent;
  • debug builds check, when a local change is committed, that the transaction linked every container its ops create. The op log cannot supply a link that a local op path forgot.

Import rollback after lazy parsing (pre-existing, #1161)

A failed import can parse old change blocks while computing its diff or validating movable-list ops (on main through those paths, now also through the resolver). Parsing registers the containers the block's ops use and allocates their values. The import rollback dropped those registrations and values but kept the parsed blocks, so their ops pointed at indices and value slices that later registrations reuse or that no longer exist. Two symptoms follow on main:

• exporting the history hit unreachable! in the JSON encoder;
• a later checkout panicked on arena.get_value(..).unwrap(), Honest import panics after an import rejected by state validation was rolled back (arena values) #1161's signature.

ChangeStore::rollback_import now drops the parsed changes of the blocks parsed since the import began, so the next access parses and registers again. A block records the arena's extent (containers, values, text) when it is parsed. A block parsed before the checkpoint can only refer to what was there then, so it stays parsed. An earlier version dropped every parsed block, which made each failed import an O(blocks) pass and the next history read a full reparse (+1937% / +258% in the performance table).

The resolver does not take the op log lock, so it can run during any import. Two rules keep that safe:

• Every arena rollback of a failed import goes through the change store. ChangeStore::rollback_arena rolls the arena back under inner (where blocks are parsed) and drops the parsed changes of the blocks parsed since the checkpoint. rollback_import ends with it. The early returns of import_changes_and_apply_delta_to_state_if_needed now use it through OpLog::rollback_arena: a decode error, and updates that depend on history before the shallow root. They used to call SharedArena::rollback directly (found in the second review round). A failed snapshot import retires the replaced store (ChangeStore::retire), so a resolver that reached it before the swap finds nothing instead of registering the discarded history.
• Every import rollback runs under the state lock. OpLog::rollback_import, rollback_arena, and reset_to_empty_for_failed_snapshot_import take &DocState as proof. A state-locked query (has_container, is_deleted, get_path_to_container) can register containers through the resolver and use their indices before it returns, and a rollback in between would free them. With the change-store part alone, the loom model below still hit get_depth on a freed index. Rolling back already happened under the state lock when state validation fails; the rollbacks at decode errors, preflight errors, movable-list validation, import_batch's reattach, and the snapshot reset's second call site now take it too.

DocState is still not rolled back: its store and dead-container cache keep entries at the freed indices, and the next container registered at one inherits them. That half is pre-existing, the same on main and here, and tracked in #1164. A related route to #1164 (I could not comment on the issue): a handler created on another thread while an import is failing keeps an index that the rollback frees. On main a later is_deleted() aborts at dead_containers_cache.rs:55, or the handler refers to another container once the index is reused. The resolver's registrations are safe from this now: the query that made them finishes before the rollback, and the next query resolves again. get_path_to_container used to unwrap its registration and now answers None if it is already gone.

Rationale and invariants: context/arena-parent-links.md, linked from the root and loro-internal AGENTS.md. It also records a testing pitfall: under cfg(test), ChangeStore::import_all parses every peer's last change at load, so loro-internal unit tests don't reproduce lazy-parsing bugs; the loro crate's tests do.

Tests

• crates/loro/tests/unregistered_container_parent.rs, each test with snapshot, fork, and shallow sources compared against an updates-imported reference:
  • is_deleted, edits (ContainerDeleted), get_path_to_container, has_container, get_container (also by ID only, in a fresh document);
  • reviving the node, then editing;
  • importing another peer's revival + edit with a root subscriber and a tree subscriber (events, paths, ancestor walk), with identical events across sources;
  • undo/redo/revert_to after the loaded document revives the node itself (the local move registers the meta's parent, so this one also passes on main), and undo of a local edit after importing another peer's revival + edit of the meta (aborts on main);
  • IDs that no op creates;
  • the review harness dump (unregistered_meta_parent.{snapshot,updates}.bin, node 7@40881) via updates, snapshot, and snapshot + fork;
  • the failed-import rollbacks: a later export (failed_import_that_loads_an_old_change) and Honest import panics after an import rejected by state validation was rolled back (arena values) #1161's missing movable-list values on a later checkout (failed_import_that_loads_old_movable_list_values);
  • queries_race_with_failing_imports: queries through the resolver against imports that fail at decode (JSON) or depend on history before a shallow root. The test times each failing import once and sweeps the query thread's start across that window, 120 trials by default. On 53a74955 (before the round-2 fix) the decode kind failed in 6/6 release and 3/4 debug runs: an existing container answered has_container = false, or the exported history differed. The shallow kind did not reproduce there (0 of 4 × 200 trials) and is kept as coverage for that early return. The fix passed 3 × 2000 trials in release and 3 × 2000 in debug;
  • queries_race_with_history_readers, a thread stress. Each trial loads a fresh document, then one thread runs is_deleted, one runs has_container / get_path_to_container, and one runs len_changes / export / frontiers_to_vv, with a watchdog. 500 trials by default (UNREGISTERED_PARENT_THREAD_TRIALS). Before the lock-order fix it deadlocked in 2 of 3 runs of 3000 trials. It also found a check-then-act race in does_container_exist: a handler created on another thread could register the container between two lookups, and has_container answered false. That check now uses its first lookup;
  • a seeded random test: 3 peers, tree create/move/delete (including under deleted parents), meta edits with nested text, filler that spreads changes over blocks, syncs; loaded via snapshot, fork, shallow at a random root, and state-only; every node's metadata (deleted flag, has_container, get_container, path, value) must equal the reference, visited in random order, and again after editing every meta and reviving some nodes. 20 seeds by default; UNREGISTERED_PARENT_SEEDS=0..300 passes in release. On main it aborts with the arena.rs panic.
• crates/loro/tests/multi_thread_test.rs: resolving_a_meta_parent_while_an_import_fails, the reviewer's loom model of has_container against a JSON import that fails at decode after registering a container. It fails on 53a74955 (the existing meta reads as missing) and passes now. With only the change-store part of the fix, it panicked in get_depth on a freed index. Also resolving_a_meta_parent_while_another_thread_reads_the_history, a loom model (pnpm test-loom) of is_deleted / has_container against len_changes / export. All three variants report deadlock on the code before the lock-order fix and pass after it.
• Unit tests:
  • arena.rs: the resolver runs outside the arena lock (one that re-enters the arena does not deadlock get_parent / get_depth); a container nobody creates has no parent, for both Loaded and Absent; an arena without a resolver still panics; a parent cycle fails fast.
  • parent.rs: the local link check fires.
  • change_store.rs: a retired store resolves nothing, and a rollback drops only the blocks parsed after the checkpoint.
• crates/loro-wasm/tests/unregistered_container_parent.test.ts: the same entry points through loro-crdt (snapshot and fork, getContainerById, getPathToContainer, revival, imported-edit events, undo after an imported edit, the dump). On a WASM build of main 5c7552dc all 6 fail: 5 with RuntimeError: unreachable (Parent is not registered), and getContainerById returns undefined.

Validation

On 76aadb57, which merges main 29553d5b (includes #1125 and #1142):

• cargo test -p loro -p loro-internal: 58 test binaries, 1320 passed, 0 failed.
• pnpm check (clippy -Dwarnings): pass. rustfmt --check on the changed files: clean.
• pnpm test-loom: 9/9, including both new models.
• UNREGISTERED_PARENT_SEEDS=0..300 cargo test --release -p loro --test unregistered_container_parent: pass.
• pnpm -C crates/loro-wasm build-release (vitest, tsc --noEmit, deno, bun): 34 files, 440 tests passed; deno 4, bun 4.
• The reviewers' evidence, re-run on this head:
  • round 1: the thread stress (3000 trials per pairing), the mixed stress (2000), and the callback re-entrancy test pass;
  • round 2: rv2_window_probe (100 trials, 42 inside the import's window, 0 bad) and rv2_resolver_races pass, the latter with all 17 partner operations in has and prereg modes (150 trials each), plus failing imports (JSON decode, state validation, old deps on a shallow source; 300 trials each). rv2_has_container_never_false: 128,000 answers, 0 false;
  • the Honest import panics after an import rejected by state validation was rolled back (arena values) #1161 harness (R1125_FORGE=1, debug) passes on the 6 seeds where main aborts and on all 13 seeds listed in Honest import panics after an import rejected by state validation was rolled back (arena values) #1161 (panics=0).
• The new Rust tests on main 29553d5b:
  • 6 abort with InternalError: Parent is not registered;
  • failed_import_that_loads_an_old_change aborts on unreachable! in json_schema.rs;
  • failed_import_that_loads_old_movable_list_values aborts on arena.get_value(..).unwrap() in history_cache.rs;
  • container_by_id_in_a_fresh_snapshot_doc gets None;
  • both thread tests fail because has_container answers false for existing metas;
  • ids_that_no_op_creates_are_not_containers (a guard) and undo_and_revert_touching_the_meta pass.

Performance

Import, export, and rollback paths (00a802eb vs main 29553d5b)

Release builds in separate target dirs, using the reviewer's round-2 harness plus a failed-import scenario. The two builds alternate over 6 rounds of 9 samples, alternating which goes first; timings are thread CPU time; 5-min loadavg 6.6–9.0. Values are ms, median of per-round medians (min of mins). The "earlier version" column is 45decad0, which dropped every parsed block on rollback (loadavg 9.2–11.2).

scenario	main	this PR	Δ	earlier version Δ
failed JSON import into a loaded doc (history already parsed)	0.033 (0.027)	0.034 (0.028)	+4.6% (noise)	+1937%
full history JSON export right after that failed import	3.120 (3.027)	3.119 (2.999)	−0.0%	+258%
import an update touching 1k alive metas into a loaded doc	2.232 (2.128)	2.252 (2.149)	+0.9%	+1.4%
import all updates into a fresh doc	23.89 (23.14)	23.49 (23.01)	−1.7%	+2.6%
load + checkout mid + back	25.27 (24.32)	24.43 (23.84)	−3.3%	−1.4%
export snapshot / shallow snapshot / all updates from a loaded doc	2.306 / 56.23 / 19.92	2.307 / 56.82 / 19.63	+0.0% / +1.0% / −1.5%
flush + export after 20k local ops (flush_and_compact)	1.073 (1.056)	1.044 (1.016)	−2.6%	−4.9%
export_json_updates (full history) from a loaded doc	11.45 (11.30)	11.56 (11.21)	+0.9%	−2.4%
is_deleted on 20k alive metas / pre-created handlers	9.73 / 8.61	9.99 / 8.85	+2.6% / +2.8%	+4.5% / +1.4%
50k local inserts into meta-nested maps	8.90 (8.70)	8.44 (8.27)	−5.1%	−6.8%

Hot paths and lookups (ba1f7092 vs main 29553d5b)

Release builds in separate target dirs, from the reviewer's harness plus one scenario (IDs beyond the history). main 29553d5b and ba1f7092 (this head minus docs) alternate over 6 rounds of 9 samples, alternating which goes first; timings are thread CPU time; 5-min loadavg 7.6–11.0. Fixture: a 20k-node tree (every 4th meta edited), 200 deleted parents with 10 materialized and 10 never-materialized children each, then 100k map ops. Every sample uses a fresh document loaded from the snapshot. Values are ms, median of per-round medians (min of mins):

scenario	main	this PR	Δ
load snapshot	0.460 (0.454)	0.463 (0.444)	+0.7%
load + first is_deleted (alive meta)	23.34 (22.21)	22.86 (22.38)	−2.1%
is_deleted on 20k alive metas	10.29 (9.90)	10.40 (9.67)	+1.0%
is_deleted on 20k pre-created meta handlers	9.00 (8.61)	8.98 (8.64)	−0.2%
get_container(meta) on 20k alive metas	11.54 (11.26)	11.60 (11.34)	+0.5%
is_deleted on 2k deleted materialized metas	1.702 (1.590)	1.698 (1.568)	−0.2%
load + checkout mid + back	25.78 (25.10)	25.10 (24.77)	−2.6%
import an update touching 1k alive metas	2.292 (2.180)	2.263 (2.197)	−1.3%
import all updates into a fresh doc	24.32 (23.82)	24.45 (23.67)	+0.5%
50k local inserts into meta-nested maps	8.46 (8.22)	8.30 (8.12)	−1.8%
has_container on 2k in-history IDs that no op creates, first pass	1.286 (1.257)	6.087 (5.962)	+373%
the same, second pass	0.659 (0.640)	0.780 (0.714)	+18%
has_container on 2k IDs beyond the history	1.094 (1.062)	2.077 (2.011)	+90%
first is_deleted / 2k is_deleted on never-materialized metas	aborts	22.90 / 1.060

Hot paths show no difference beyond noise. Lookups of normal IDs that are not registered now ask the history:

• An ID beyond the history costs a KV scan and one block header, about 0.5 µs, with no parse.
• An in-history ID whose op creates no container costs one parse of the block that holds the op. The block stays cached, as after any lazy access to that part of the history. Only the op's content tells whether it creates the container, so a version-vector or DAG pre-check cannot avoid that parse.

A vv shortcut for out-of-history IDs would have to be exact for every path that writes the store's KV, or a live container would read as deleted, so I left it out. This is documented in the context file.

🤖 Generated with Claude Code

[github-actions]

WASM Size Report

• Original size: 3277.59 KB
• Gzipped size: 1089.33 KB
• Brotli size: 762.83 KB

[zxch3n]

Changes after the adversarial review of 021639af, now at 53a74955 (merges main 29553d5b):

B1, the deadlock (blocking). ChangeStore now takes its locks in one order, root_history_names → external_kv → inner → external_vv, then the arena's lock. The order is documented on the struct and in context/arena-parent-links.md. Flipping only get_parsed_block_in was not enough. Four places took inner before external_kv or cloned it under inner:

• get_parsed_block_in, the resolver's path, now load_parsed_block. It looks for a cached block under inner alone, then takes external_kv and inner in order and looks again before loading.
• flush_and_compact.
• get_change_by_lamport_lte, from fix: reject malformed movable list move/set ops on import instead of aborting #1125.
• ChangeStore::fork, which cloned external_kv while holding inner.

I chose one order over "the resolver never holds both locks": with that variant, re-locking inner to insert could cache a stale copy of a block whose newer version another thread had meanwhile flushed and a failed import had evicted.

Regressions:

• the loom model from the review, in multi_thread_test.rs (pnpm test-loom), covering is_deleted / has_container against len_changes / export. All three variants report deadlock before the fix and pass after it.
• queries_race_with_history_readers, a thread stress with a watchdog. Before the fix it deadlocked in 2 of 3 runs of 3000 trials.

Your stress tests pass on this head: 3000 trials for each pairing, 2000 for the mixed stress with a reference.

Found while fixing B1:

• A failed import rolled back the change store, then the arena. The resolver runs without the op log lock, so it could parse a block between the two steps and keep indices that the arena rollback then drops. ChangeStore::rollback_import now rolls back the arena under inner.
• The stress test also caught a check-then-act race in does_container_exist: a handler created on another thread registered the meta between two lookups, and has_container answered false. It now decides from its first lookup.

Non-blocking:

1. DocState rollback: filed as A failed import leaves DocState entries at arena indices its rollback freed #1164, with your minimal repro. It is the same on main 29553d5b and this head. The context file's "Import rollback" section now says that DocState is not rolled back.
2. Honest import panics after an import rejected by state validation was rolled back (arena values) #1161: linked as "Fixes Honest import panics after an import rejected by state validation was rolled back (arena values) #1161". failed_import_that_loads_old_movable_list_values is a small deterministic case. It aborts on main at history_cache.rs:992 (arena.get_value(..).unwrap()), the Honest import panics after an import rejected by state validation was rolled back (arena values) #1161 signature. Your Honest import panics after an import rejected by state validation was rolled back (arena values) #1161 harness passes on this head for main's 6 failing seeds and all 13 seeds listed in the issue.
3. Lookup cost: documented in the context file and the PR description, with a new scenario for IDs beyond the history (+90%, about 0.5 µs, no parse). In-history IDs still cost one parse per block (+373% first pass, +18% second). A vv or DAG pre-check can tell only whether the op is in the history, not whether it creates a container, so it cannot avoid that parse. A store-side vv shortcut for out-of-history IDs would have to be exact for every KV write, or a live container would read as deleted, so I left it out.
4. Fail-fast:
   • The resolver answers Loaded or Absent and panics when a block holds the op but cannot be parsed.
   • ContainerWrapper::new names its invariant: no state without a parent.
   • Debug builds check on commit that the transaction linked every container a local op creates. That is where a forgotten link would originate, and the resolver cannot supply one because the op is not in the store yet. The full suite passes with the check on.
5. get_depth panics on a parent cycle (unit test a_parent_cycle_fails_fast).

Validation and a performance re-run (6 alternating rounds against 29553d5b; hot paths within noise) are in the PR description.

[zxch3n]

Changes after the second review round of 53a74955, now at 76aadb57 (main 29553d5b, unchanged):

R2-B1 (blocking), the early arena rollbacks. Every arena rollback of a failed import now goes through the change store: ChangeStore::rollback_arena rolls back under inner and drops the parsed changes of the blocks parsed since the checkpoint. That covers the three early returns of import_changes_and_apply_delta_to_state_if_needed (decode error, and the two deps-before-shallow-root returns). A failed snapshot import retires the replaced store (ChangeStore::retire), so a resolver that reached it before the swap answers Absent instead of registering the discarded history. This also covers the reset's second call site.

The change-store part alone was not enough. With it, your loom model (reproduced in multi_thread_test.rs::resolving_a_meta_parent_while_an_import_fails) still panicked in get_depth on a freed index. has_container had registered the meta through the resolver and was still using the index when the other thread's early rollback freed it. So every import rollback now also runs under the state lock: OpLog::rollback_import, rollback_arena, and reset_to_empty_for_failed_snapshot_import take &DocState as proof. The sites that rolled back without it now take it in the op log → state order:

• the decode and preflight errors;
• the movable-list validation failures, detached and attached;
• _import_with's error paths;
• import_batch's reattach rollback;
• the snapshot reset's second call site.

Regressions:

• the loom model above. It fails on 53a74955 (the meta reads as missing) and passes now.
• queries_race_with_failing_imports. Each failing import (JSON that fails to decode; updates depending on history before a shallow root) is timed once, and the query thread's start is swept across that window. On 53a74955 it failed in 6/6 release and 3/4 debug runs. The fix passed 3 × 2000 trials in release and 3 × 2000 in debug. The shallow kind did not reproduce on 53a74955 (0 of 4 × 200). As far as I can tell, a shallow document keeps these containers in its state, so the resolver only parses there for IDs that are not containers. It stays as coverage for that early return.

Your round-2 probes pass on this head: rv2_window_probe (0 bad), rv2_resolver_races with all 17 partner operations in has and prereg modes plus the failing imports, and rv2_has_container_never_false (0 false out of 128,000). The #1161 harness passes all 18 seeds.

A regression my first R2-B1 fix introduced, now fixed. Dropping the parsed changes of every cached block on each failed import made the failed import itself +1937% and the next history read +258% (a full reparse). A peer sending malformed updates could force that repeatedly. Blocks now record the arena's extent when they are parsed, and the rollback drops only blocks whose extent goes past the checkpoint. Both scenarios are back to main (+4.6% noise and −0.0%). Import, export, flush, and checkout are within ±3%.

Non-blocking:

1. get_path_to_container no longer unwraps; it answers None if a failed import on another thread already rolled back its registration. I could not comment on A failed import leaves DocState entries at arena indices its rollback freed #1164 ("Resource not accessible by integration"), so the handler route (a handler created during a failing import keeps a freed index; dead_containers_cache.rs:55 on main) is written up in the PR description and in context/arena-parent-links.md.
2. Panicking on an unparsable block is noted in the changeset and the context file.

Validation: cargo test -p loro -p loro-internal 1320/0, pnpm check, pnpm test-loom 9/9, random 0..300, WASM 440, all on 76aadb57.