Skip to content

C-list import accounting is asymmetric: the refcount increment on import is missing #1006

Description

@grypez

Summary

The kernel's c-list reference accounting is asymmetric: the decrement half is implemented and the increment half is missing. Creating an import c-list entry changes no refcount; tearing one down decrements both reachable and recognizable.

Compensating for this, initKernelObject births every kernel object at (1, 1) instead of SwingSet's (0, 0). That constant makes the arithmetic come out exactly right for a single importer, which is why the whole kernel works and why no test catches this. It cannot stand in for a second importer, and it is claimed by two different parties in our own code.

All symptoms below were reproduced against the real store (makeKernelStore + makeMapKernelDatabase), not inferred. Traces are verbatim. Line numbers are against main.

The asymmetry

Path reachable recognizable
addCListEntrystore/methods/clist.ts:34, hardcodes isReachable=true at :37
clearReachableFlagstore/methods/reachable.ts:63 −1
deleteCListEntrystore/methods/clist.ts:60 −1 (via clearReachableFlag) −1 (onlyRecognizable: true)

There is no setReachableFlag anywhere in the repo, and git log --all -S setReachableFlag is empty — it was never ported, not removed. addCListEntry hardcodes the flag at creation and nothing ever sets it again.

The import path is translateRefKtoE (store/methods/translators.ts:64) → allocateErefForKrefaddCListEntry. No incrementRefCount appears anywhere along it.

Upstream SwingSet has both halves in vatKeeper: mapKernelSlotToVatSlot increments with {onlyRecognizable: true} on allocating an import entry, then calls setReachableFlag to bump reachable.

Why this has been invisible

For one exporter and one importer the two schemes agree:

birth importer's c-list entry total
SwingSet (0,0) {onlyRecognizable} +1 rec, then setReachableFlag +1 reach (1,1)
ocap-kernel (1,1) nothing (1,1)

Our export path is a faithful port and is not implicated: exportFromEndpoint (store/methods/vat.ts:445) passes {isExport: true, onlyRecognizable: true}, which store/methods/refcount.ts:99 short-circuits for objects — a no-op, exactly as in SwingSet. collectGarbage (store/methods/gc.ts) is likewise a faithful port of processRefcounts. The defect is confined to c-list accounting; the surrounding machinery is correct and is consuming wrong inputs.

packages/kernel-test/src/garbage-collection.test.ts has two cases and neither involves a second importer, so nothing end-to-end exercises the divergence.

Symptoms

1. Two importers → a live capability is dropped and retired

v1 export (1,1) -> v2 import (1,1) -> v3 import (1,1)   [neither import incremented]
v2 dropImports  -> (0,1)
v2 retireImports-> (0,0)                                [v3 still isReachable=true]
collectGarbage() emits: ["v1 dropExport ko1","v1 retireExport ko1"]

The still-live owner is told to drop and retire an object v3 legitimately holds. collectGarbage consults only the owner's reachable flag (store/methods/gc.ts:162), so it cannot see v3. shouldProcessAction (garbage-collection/garbage-collection.ts:52) re-validates at delivery time but consults the same owner-only view, so the negation guard is inert here. In SwingSet the guard works precisely because a second import raises reachable.

2. Refcount underflow inside vat cleanup

Simplest form — one importer, one drop, one termination. No queued message required:

v1 exports          (1,1)
v2 imports          (1,1)
v2 dropImports ONLY (0,1)
v1 cleanupTerminatedVat: Error: "ko1" underflow -1,0

The throw comes from setObjectRefCount's guard (store/methods/object.ts:131) and escapes from the middle of cleanupTerminatedVat's export loop, leaving the vat half-cleaned.

The variant originally filed here also reproduces, via a queued message:

v1 exports (1,1); v2 drop+retire -> (0,0)
message carrying ko1 enqueued    -> (1,1)   [KernelQueue.ts:274 'queue|slot']
v1 terminates, cleanup decrements-> (0,0)   [steals the queued message's ref]
that message is later delivered:
  KernelRouter decrementRefCount(slot, 'deliver|send|slot')
  Error: "ko1" underflow -1,-1

3. Re-import after drop never restores reachability

performDropImports (garbage-collection/gc-handlers.ts:26) only calls clearReachableFlag, so the c-list entry survives at isReachable=false. On re-delivery, translateRefKtoE returns the existing eref via krefToEref, which parses out vatSlot and discards isReachable (store/methods/clist.ts:131):

v2 import            (1,1)  isReachable=true
v2 dropImports only  (0,1)  isReachable=false   [c-list entry retained]
v2 RE-import         (0,1)  isReachable=false   [same eref, nothing restored]

SwingSet's mapKernelSlotToVatSlot calls setReachableFlag on every K→V import translation, restoring both flag and count; GC deliveries deliberately opt out via {setReachable: false}. We have no equivalent, and no insistNotReachable-style assertion either, so this is silent. The object stays flagged unreachable in the kernel while the vat holds it live — GC is then free to retire it underneath a live holder, and a later retireImports from that vat passes its own guard (gc-handlers.ts) because the flag is still false.

4. Objects leak when a message goes splat

The mirror-image failure, which over-collection framing misses:

v1 exports            (1,1)
enqueued              (2,2)
splat, no importer    (1,1)
GC actions: []   object still exists: true   importers: []

(1,1) with zero holders, permanently. SwingSet reaches (0,0) and collects. Any invariant checker that only looks for underflow will bless this, so it has to check both directions.

Root cause: the baseline is claimed by two parties

The (1,1) unit has two mutually exclusive readings, and both are implemented:

  • Importer-sidestore/methods/object.ts:21-29: born at 1 "on the assumption that the new object corresponds to an object that has just been imported from somewhere." Consistent with an importer's drop decrementing it (store/methods/reachable.ts, store/methods/clist.ts).
  • Owner-sidestore/methods/vat.ts:391: "The baseline decrement below corresponds to the implicit reference exportFromEndpoint installed when the kernel object was first created," released at vat.ts:265 and vat.ts:419.

Both an importer's drop and the owner's termination are entitled to spend the same single unit. That double-claim — not the missing increment alone — is what turns an undercount into the symptom-2 crash.

The owner-side reading additionally rests on a deposit that never happens: vat.ts:445 passes isExport: true, which refcount.ts:99 short-circuits for objects. The withdrawal at vat.ts:265 is drawing on the birth baseline instead.

For contrast, the promise baseline is coherent and is the shape objects need: initKernelPromise sets 1 (store/methods/promise.ts:49), and that unit is released at exactly one well-defined event, decrementRefCount(kpid, 'resolve|decider') (store/methods/promise.ts:172). One creation, one release.

Note also that store/methods/gc.ts:168-169 contains a commented-out consistency check on exactly this invariant, with // TODO: rethink this assert.

Adjacent gap: GC action delivery mutates no kernel state

Independent of the increment, worth fixing in the same area. #deliverGCAction (KernelRouter.ts:406) only translates and delivers. SwingSet's vatTranslator additionally does clearReachableFlag on dropExports and deleteCListEntry on retireExports/retireImports.

Consequences: the owner's R flag never clears after a dropExport (so the action can be re-derived), and importer c-list entries survive retirement — verified: after retireKernelObjects, the refCount row is gone while hasCListEntry still returns true. Those lingering rows are part of what makes the symptom-2 underflow reachable.

Relatedly, krefsToExistingErefs (KernelRouter.ts:412) silently filters unmapped krefs where SwingSet maps GC krefs with required: true and would Fail.

Where this came from

Corrected history:

  • The (1,1) baseline is not part of the DGC port. It predates it by five months — 9739db1ca ("Implement kernel storage abstraction layer", Implement kernel storage abstraction layer #180, Chip Morningstar, 2024-10-24), originally a single kv.set(refCountKey(koId), '1'), with the "imported from somewhere" comment already attached. aa7e99490 ("Add reachability tracking") converted it mechanically to {reachable: 1, recognizable: 1}.

  • The DGC port is PR feat(kernel): Support liveslots distributed garbage collection #419 / commit 2f6e0d309 (Implement kernel-side support for liveslots distributed garbage collection #329 is the issue it closes, not a PR). Its own commit message states the intent as:

    update these refcounts during clist additions/removals

    The removals landed; the additions did not. On that commit's own terms the increment half is an oversight, not a design decision. No comment, commit message, PR review, or doc anywhere argues for an intentional divergence from SwingSet.

So the baseline was a deliberate, reasonable pre-GC decision that happens to mask a half-finished port for the single-importer topology our tests exercise.

Note on the previously-cited reachable > 0 guard

An earlier revision of this issue described a reachable > 0 guard around the baseline decrement in cleanupTerminatedVat and called it "dead code as merged." Correction: PR #983 is still open. main has the unconditional decrementRefCount(kref, 'cleanup|export|baseline') at store/methods/vat.ts:265 (added by 690dec010b, #492). There is nothing merged to revert.

The substantive objection to that guard still holds, and was checked by applying it by hand: it tests the wrong predicate. Symptom 2 reproduces with the guard in place, because reachable is non-zero at the moment cleanup runs. It also skips recognizable, and it disagrees with the sibling unguarded decrement at vat.ts:419.

Note on #994

#994 states that translateRefKtoE(remoteId, kref, true) "allocates a c-list entry and increments the refcount," and lists a pinned refcount as its first consequence. No increment occurs on that path, so that particular consequence is unfounded. #994's other two consequences (unbounded c-list growth, existence oracle) are unaffected and stand.

There is an inverse risk there worth adding: the remote receives a c-list entry at isReachable=true with no ref backing it, so it can later be handed a retireImport for something it believes it holds.

Proposed path

Step 1 — build a refcount invariant checker, first.
Recompute expected (reachable, recognizable) for every kref from ground truth — c-list entries and their reachable flags, run-queue message slots, promise resolution slots, pins — and assert against stored counts. Run after every crank behind a debug/test flag.

It must flag both directions: counts too low (symptoms 1–3) and counts too high with no holder (symptom 4). Underflow-only checking would pass symptom 4.

This is worth doing on its own merits. It replaces "patched against e2e tests" with a checkable invariant, measures the true blast radius before anyone commits to a fix, and identifies which existing guards are compensating for the missing increment.

Step 2 — restore the increment and pick one meaning for the baseline.
Recommended: match SwingSet. initKernelObject(0,0), add the increment on import c-list creation, add setReachableFlag, and delete the owner-side baseline decrements at vat.ts:265 and vat.ts:419. Our collectGarbage is already a faithful port of processRefcounts, so this makes it receive the inputs it was written for, and keeps future cribbing from SwingSet safe.

The alternative — keep the baseline as an explicit owner credit and add the per-import increment on top — is a smaller diff but a permanent divergence, and requires rewriting object.ts's comment plus auditing that nothing importer-side ever spends it.

Two costs to price in:

  • Refcounts are persisted. Changing the accounting invalidates existing kernel stores; needs a migration or a recompute-from-c-lists pass at open.
  • The increment cannot land alone. Every existing compensation becomes a double-count the moment it does, so the audit-and-remove has to land in the same change. Step 1 is what makes that audit tractable. Note store/methods/object.test.ts asserts (1,1) directly and will move.

Step 3 — once the invariant holds, delete the sibling compensations, re-enable the commented-out assert at gc.ts:169, and close the GC-delivery gap above.

Acceptance

  • Invariant checker exists and runs in test mode; violations fail the build
  • Checker detects counts that are too high with no holder, not just underflow
  • Symptoms 1–4 each have a regression test
  • cleanupTerminatedVat has direct unit coverage (it currently has none — only a name-export assertion at store/index.test.ts)
  • Multi-importer topology (≥3 endpoints sharing one object) covered in kernel-test
  • vat.ts:265 and vat.ts:419 agree on baseline-decrement semantics
  • initKernelObject's doc comment states which party the baseline belongs to (or is deleted with the baseline)
  • GC action delivery clears the owner's reachable flag and tears down retired c-list entries

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions