Skip to content

A failed import leaves DocState entries at arena indices its rollback freed #1164

Closed

Description

@zxch3n

Summary

When an import fails state validation and is rolled back, OpLog::rollback_import rolls back the arena, the change store, pending changes, and the DAG, but not DocState. The state store (InnerStore wrappers loaded while validating, e.g. through validate_diff_batch / needs_checkout_diff → store.get_container(idx)) and dead_containers_cache keep entries keyed by the arena indices the rollback freed. The next container registered at a freed index inherits the old state.

This was found while reviewing #1159. It predates that PR and happens on main 29553d5b and on #1159 alike. It is the other half of the #1161 family: #1159 fixes the change-store half, where lazily parsed blocks kept indices past the truncated arena.

Reproduction (deterministic, single thread)

Add this as crates/loro/tests/failed_import_state_repro.rs and run cargo test -p loro --test failed_import_state_repro.

use loro::{ExportMode, LoroDoc, LoroMap, LoroText, ToJson, TreeParentId};

fn import(bytes: &[u8]) -> LoroDoc {
    let doc = LoroDoc::new();
    doc.import(bytes).unwrap();
    doc.set_peer_id(99).unwrap();
    doc
}

#[test]
fn failed_import_leaves_state_at_freed_indices() {
    // Peer 2 creates `c` under `p` and puts a text in its meta; peer 1 deletes `p` concurrently.
    let a = LoroDoc::new();
    a.set_peer_id(1).unwrap();
    let p = a.get_tree("tree").create(TreeParentId::Root).unwrap();
    a.commit();
    let b = LoroDoc::new();
    b.set_peer_id(2).unwrap();
    b.import(&a.export(ExportMode::all_updates()).unwrap()).unwrap();
    let c = b.get_tree("tree").create(p).unwrap();
    let meta = b.get_tree("tree").get_meta(c).unwrap();
    meta.insert_container("t", LoroText::new()).unwrap().insert(0, "z").unwrap();
    b.commit();
    a.get_tree("tree").delete(p).unwrap();
    a.commit();
    a.import(&b.export(ExportMode::all_updates()).unwrap()).unwrap();
    a.get_map("m").insert("k", 1).unwrap();
    a.commit();
    let snap = a.export(ExportMode::Snapshot).unwrap();
    let vv = a.oplog_vv();

    // Peer 4 revives `c` and edits its meta, then a list insert forged out of bounds, which
    // the state rejects.
    let e = LoroDoc::new();
    e.set_peer_id(4).unwrap();
    e.import(&snap).unwrap();
    e.get_tree("tree").mov(c, TreeParentId::Root).unwrap();
    e.get_tree("tree").get_meta(c).unwrap().insert("r", 1).unwrap();
    e.get_list("list").insert(0, 1).unwrap();
    e.get_list("list").insert(1, 2).unwrap();
    e.commit();
    let mut json = serde_json::to_value(e.export_json_updates(&vv, &e.oplog_vv())).unwrap();
    let last = json["changes"].as_array_mut().unwrap().last_mut().unwrap()["ops"]
        .as_array_mut()
        .unwrap()
        .last_mut()
        .unwrap();
    last["content"]["pos"] = 1000.into();
    let carrier = LoroDoc::new();
    carrier.import(&snap).unwrap();
    carrier.detach();
    carrier.import_json_updates(serde_json::to_string(&json).unwrap()).unwrap();
    let bad = carrier.export(ExportMode::updates(&vv)).unwrap();

    let doc = import(&snap);
    let reference = import(&snap);
    assert!(doc.import(&bad).is_err());
    for d in [&doc, &reference] {
        d.get_map("fresh")
            .insert_container("c0", LoroMap::new())
            .unwrap()
            .insert("v", 1)
            .unwrap();
        d.commit();
    }
    // Fails: `fresh` also shows `"t": "z"` from `c`'s meta.
    assert_eq!(
        doc.get_deep_value().to_json_value(),
        reference.get_deep_value().to_json_value()
    );
}

Result on main 29553d5b:

  left: {"fresh": {"c0": {"v": 1}, "t": "z"}, "m": {"k": 1}, "tree": []}
 right: {"fresh": {"c0": {"v": 1}}, "m": {"k": 1}, "tree": []}

The new root map fresh got the arena index that the rolled-back import had given to c's meta, and DocState still holds the meta's state under that index.

Threaded variant

The #1159 reviewer also ran a threaded version: thread A queries metas while thread B's bad import fails twice, then 10 new containers are created. On main, 47/100 and 51/100 trials were wrong: new containers reported ContainerDeleted, and the root fresh read as deleted. Some runs on both main and #1159 aborted with a stack overflow, from a container that references itself after its index was reused. Other threads can register containers during an import's window (handlers, root lookups, and, with #1159, the op log's creator resolver), and the rollback drops those registrations too.

Direction

Either DocState needs its own rollback for entries at indices at or above the arena checkpoint (the store wrappers, dead_containers_cache, and anything else keyed by ContainerIdx), or the arena must not reuse rolled-back indices while DocState may still refer to them.

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions