Conversation
Multiple Go-level *Array or *OrderedMap instances created over the same logical container held independent root slab pointers. A structural change (splitRoot, promoteChildAsNewRoot) performed through one instance left siblings observing stale roots. Hoist the mutable state onto a shared *arrayState / *orderedMapState that lives in a per-SlabStorage registry keyed by root SlabID. All instances over the same logical container share one state pointer, so structural changes propagate automatically. parentUpdater stays per-instance: a regular Get installs a real parent-notification callback while a readonly iterator installs a trap callback; two siblings can legitimately have different parentUpdaters and a mutation must fire only the originating instance's callback. SlabStorage gains a state registry. External implementations can embed *BaseStateRegistry to satisfy them with zero boilerplate. BasicSlabStorage and PersistentSlabStorage do exactly this.
add more tests for sibling
Add mutation count
Eliminate stale-sibling-root inconsistency
A registered container state deliberately survives storage.Remove because Remove is also called when a container is inlined while still alive. The side effect: after a container was genuinely destroyed, the constructors served the leftover state as a live container (a zombie), where they previously returned SlabNotFoundError. Distinguish the two cases on lookup: a non-inlined root must still have its slab in storage. If the slab is gone, the container was destroyed — clear the leftover state and return SlabNotFoundError, restoring pre-shared-state behavior. Inlined roots are exempt since they legitimately have no standalone slab. The existence check only fires on a registry hit with a non-inlined root; a cold-cache hit costs one base-storage read + decode, which is what the constructors paid on every call before the shared-state registry existed. Adds StateRegistry.RemoveStateForSlab so the constructors can clear the stale entry through SlabStorage; BaseStateRegistry already implements it, so embedders are unaffected.
When the shared-state registry was introduced, NewMapWithRootID lost the check that the slab carries extra data; its array counterpart kept it. Only root slabs carry extra data — a slab without it is an interior slab, not a value. Without the check, passing an interior slab ID silently produced a broken *OrderedMap with an unseeded digester, where it previously returned NotValueError. Worse, the bogus state was registered in the registry under that slab ID, poisoning every later lookup for it. Reject the slab before registering state, so a failed call leaves the registry untouched. The new tests cover both container types: - the map test is the regression test, - the array test pins the existing check against the same mistake.
DropDeltas rolls storage back to the last committed state, but registered container states point at in-memory root slabs that still reflect the discarded mutations. An *Array / *OrderedMap instance created after the rollback would be served such a state from the registry and resurrect the discarded writes. Add BaseStateRegistry.RemoveAllStates and call it from DropDeltas, so post-rollback instances re-decode the committed slabs. Note that DropDeltas alone still leaves in-place-mutated slab structs in the read cache; callers wanting a full reset to committed state pair it with DropCache (as cmd/smoke does), and the new test follows that pattern.
DropDeltas cleared deltas and the state registry, but slabs are mutated in place and the same mutated structs also sit in the read cache (commit places stored slab structs in the cache, and cached reads can be mutated afterwards). A fresh container load after the rollback would read the mutated struct from the cache and observe — and on a later commit re-persist — the supposedly discarded mutations, so DropDeltas alone was not a complete rollback and silently required pairing with DropCache. Every mutated slab has a delta entry, so invalidating the cache for all delta'd IDs makes subsequent reads decode the committed bytes again. DropDeltas alone is now a complete rollback for everything storage serves afterwards. One vector remains and cannot be closed in atree: container instances obtained before the rollback still hold their mutated in-memory roots. Document on DropDeltas that callers must discard retained container references across a rollback. The test now deliberately omits DropCache and asserts mutated slabs are no longer served from memory.
Follow up to #21
NewArrayWithRootID and NewMapWithRootID only prove access to standalone root slabs. After a container is inlined into its parent, the old root ID no longer names a live standalone slab, even though the shared-state registry must keep the state alive for parent-loaded siblings. Reject constructor calls that hit an inlined registered state without clearing that state. Also mark root-ID-loaded wrappers and reject later mutations through them if another sibling has since inlined the container and the wrapper has no parent updater. This prevents silent in-memory mutations that cannot be written back to the parent while leaving normal parent-loaded and standalone Cadence paths intact. Add array and map regressions covering existing root-ID handles, fresh root-ID loads after inline, registry retention, and continued mutation through the parent-loaded handle.
Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>
Reject root-ID access to inlined containers
zhangchiqing
approved these changes
Jul 2, 2026
janezpodhostnik
approved these changes
Jul 2, 2026
turbolent
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mainbranchFiles changedin the Github PR explorer