Skip to content

Commit

Permalink
retire abandoned unreachables (#8695)
Browse files Browse the repository at this point in the history 
fix(swingset): retire unreachable orphans

If a kernel object ("koid", the object subset of krefs) is
unreachable, and then becomes orphaned (either because the owning vat
was terminated, or called `syscall.abandonExports`, or was upgraded
and the koid was ephemeral), then retire it immediately.

The argument is that the previously-owning vat can never again talk
about the object, so it can never become reachable again, which is
normally the point at which the owning vat would retire it. But
because the owning vat can't retire it by itself, the kernel needs to
do the retirement on its behalf.

We now consolidate retirement responsibilities into
processRefcounts(): when terminateVat or syscall.abandonExports use
abandonKernelObjects() to mark a kref as orphaned, it also adds the
kref to maybeFreeKrefs, and then processRefcounts() is responsible for
noticing the kref is both orphaned and unreachable, and then notifying
any importers of its retirement.

I double-checked that cleanupAfterTerminatedVat will always be
followed by a processRefcounts(), by virtue of either being called
from processDeliveryMessage (in the crankResults.terminate clause), or
from within a device invocation syscall (which only happens during a
delivery, so follows the same path). We need this to ensure that any
maybeFreeKrefs created by the cleanup's abandonKernelObjects() will
get processed promptly.

Changes getObjectRefCount to tolerate deleted krefs (missing
`koNN.refCount`) by just returning 0,0. This fixes a potential kernel
panic in the new approach, when a kref is recognizable by one vat but
only reachable by a send-message on the run-queue, then becomes
unreachable as that message is delivered (the run-queue held the last
strong reference), if the target vat does syscall.exit during the
delivery. The decref pushes the kref onto maybeFreeKrefs, the
terminateVat retires the merely-recognizable now-orphaned kref, then
processRefcounts used getObjectRefCount() to grab the refcount for the
now-retired (and deleted) kref, which asserted that the koNN.refCount
key still existed, which didn't.

This occured in zoe - secondPriceAuction -- valid input , where the
contract did syscall.exit in response to a timer wake() message sent
to a single-use wakeObj.

closes #7212
  • Loading branch information
@mergify
mergify[bot] authored Aug 14, 2024
2 parents 74a89a7 + 9dcdabb commit 7f12e80
Show file tree
Hide file tree
Showing 10 changed files with 666 additions and 110 deletions.
5 changes: 1 addition & 4 deletions packages/SwingSet/src/kernel/kernel.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -980,10 +980,7 @@ export default function buildKernel(
const abandonedObjects = [
...kernelKeeper.enumerateNonDurableObjectExports(vatID),
];
for (const { kref, vref } of abandonedObjects) {
/** @see translateAbandonExports in {@link ./vatTranslator.js} */
vatKeeper.deleteCListEntry(kref, vref);
/** @see abandonExports in {@link ./kernelSyscall.js} */
for (const { kref } of abandonedObjects) {
kernelKeeper.orphanKernelObject(kref, vatID);
}

Expand Down
13 changes: 1 addition & 12 deletions packages/SwingSet/src/kernel/kernelSyscall.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,24 +178,13 @@ export function makeKernelSyscallHandler(tools) {

function retireExports(koids) {
Array.isArray(koids) || Fail`retireExports given non-Array ${koids}`;
const newActions = [];
for (const koid of koids) {
const importers = kernelKeeper.getImporters(koid);
for (const vatID of importers) {
newActions.push(`${vatID} retireImport ${koid}`);
}
// TODO: decref and delete any #2069 auxdata
kernelKeeper.deleteKernelObject(koid);
}
kernelKeeper.addGCActions(newActions);
kernelKeeper.retireKernelObjects(koids);
return OKNULL;
}

function abandonExports(vatID, koids) {
Array.isArray(koids) || Fail`abandonExports given non-Array ${koids}`;
for (const koid of koids) {
// note that this is effectful and also performed outside of a syscall
// by processUpgradeVat in {@link ./kernel.js}
kernelKeeper.orphanKernelObject(koid, vatID);
}
return OKNULL;
Expand Down
158 changes: 124 additions & 34 deletions packages/SwingSet/src/kernel/state/kernelKeeper.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ export const CURRENT_SCHEMA_VERSION = 2;
// $vatSlot is one of: o+$NN/o-$NN/p+$NN/p-$NN/d+$NN/d-$NN
// v$NN.c.$vatSlot = $kernelSlot = ko$NN/kp$NN/kd$NN
// v$NN.vs.$key = string
// v$NN.meter = m$NN // XXX does this exist?
// old (v0): v$NN.reapInterval = $NN or 'never'
// old (v0): v$NN.reapCountdown = $NN or 'never'
// v$NN.reapDirt = JSON({ deliveries, gcKrefs, computrons }) // missing keys treated as zero
Expand Down Expand Up @@ -621,7 +620,9 @@ export default function makeKernelKeeper(

function getObjectRefCount(kernelSlot) {
const data = kvStore.get(`${kernelSlot}.refCount`);
data || Fail`getObjectRefCount(${kernelSlot}) was missing`;
if (!data) {
return { reachable: 0, recognizable: 0 };
}
const [reachable, recognizable] = commaSplit(data).map(Number);
reachable <= recognizable ||
Fail`refmismatch(get) ${kernelSlot} ${reachable},${recognizable}`;
Expand Down Expand Up @@ -709,13 +710,32 @@ export default function makeKernelKeeper(
return owner;
}

function retireKernelObjects(koids) {
Array.isArray(koids) || Fail`retireExports given non-Array ${koids}`;
const newActions = [];
for (const koid of koids) {
const importers = getImporters(koid);
for (const vatID of importers) {
newActions.push(`${vatID} retireImport ${koid}`);
}
deleteKernelObject(koid);
}
addGCActions(newActions);
}

function orphanKernelObject(kref, oldVat) {
// termination orphans all exports, upgrade orphans non-durable
// exports, and syscall.abandonExports orphans specific ones
const ownerKey = `${kref}.owner`;
const ownerVat = kvStore.get(ownerKey);
ownerVat === oldVat || Fail`export ${kref} not owned by old vat`;
kvStore.delete(ownerKey);
const { vatSlot: vref } = getReachableAndVatSlot(oldVat, kref);
kvStore.delete(`${oldVat}.c.${kref}`);
kvStore.delete(`${oldVat}.c.${vref}`);
addMaybeFreeKref(kref);
// note that we do not delete the object here: it will be
// collected if/when all other references are dropped
// retired if/when all other references are dropped
}

function deleteKernelObject(koid) {
Expand Down Expand Up @@ -931,6 +951,14 @@ export default function makeKernelKeeper(
*
*/
function cleanupAfterTerminatedVat(vatID, budget = undefined) {
// this is called from terminateVat, which is called from either:
// * end of processDeliveryMessage, if crankResults.terminate
// * device-vat-admin (when vat-v-a does adminNode.terminateVat)
// (which always happens inside processDeliveryMessage)
// so we're always followed by a call to processRefcounts, at
// end-of-delivery in processDeliveryMessage, after checking
// crankResults.terminate

insistVatID(vatID);
let cleanups = 0;
const work = {
Expand Down Expand Up @@ -999,19 +1027,8 @@ export default function makeKernelKeeper(
const vref = stripPrefix(clistPrefix, k);
assert(vref.startsWith('o+'), vref);
const kref = kvStore.get(k);
// we must delete the c-list entry, but we don't need to
// manipulate refcounts like the way vatKeeper/deleteCListEntry
// does, so just delete the keys directly
// vatKeeper.deleteCListEntry(kref, vref);
kvStore.delete(`${vatID}.c.${kref}`);
kvStore.delete(`${vatID}.c.${vref}`);
// if this object became unreferenced, processRefcounts will
// delete it, so don't be surprised. TODO: this results in an
// extra get() for each delete(), see if there's a way to avoid
// this. TODO: think carefully about other such races.
if (kvStore.has(`${kref}.owner`)) {
orphanKernelObject(kref, vatID);
}
// note: adds to maybeFreeKrefs, deletes c-list and .owner
orphanKernelObject(kref, vatID);
work.exports += 1;
if (spend()) {
logWork();
Expand Down Expand Up @@ -1484,21 +1501,51 @@ export default function makeKernelKeeper(
return false;
}

// TODO (#9888): change the processRefcounts maybeFreeKrefs
// iteration to handle krefs that get added multiple times (while
// we're iterating), because we might do different work the second
// time around. The concerning scenario is:
//
// * kp2 has resolution data which references ko1
// * somehow both kp2 and ko1 end up on maybeFreeKrefs, but ko1.reachable=1
// (via kp2)
// * our iterator visits ko1 first, sees it is still reachable, ignores it
// * then the iterator visits kp2, decrefs its kp.data.slots
// * this pushes ko1 back onto maybeFreeKrefs
// * we need to examine ko1 again, so it can be released
//
// It could also happen if/when we implement #2069 auxdata:
//
// * ko2 has auxdata that references ko1
// * both ko1 and ko2 end up on maybeFreeKrefs, but ko1.reachable = 1
// (via the ko2 auxdata)
// * our iterator visits ko1 first, sees it is still reachable, ignores it
// * then the iterator visits ko2, does deleteKernelObject
// * this frees the auxdata, which pushes ko1 back onto maybeFreeKrefs
// * we need to examine ko1 again, so it can be released
//
// We should use something like an ordered Set, and a loop that does:
// * pop the first kref off
// * processes it (maybe adding more krefs)
// * repeats until the thing is empty
// Or maybe make a copy of maybeFreeKrefs at the start, clear the
// original, and wrap this in a loop that keeps going until
// maybeFreeKrefs is still empty at the end. Be sure to imagine a
// very deep linked list: don't just process it twice, keep
// processing until there's nothing left to do, otherwise we'll be
// leaving work for the next delivery.

function processRefcounts() {
if (enableKernelGC) {
const actions = getGCActions(); // local cache
// TODO (else buggy): change the iteration to handle krefs that get
// added multiple times (while we're iterating), because we might do
// different work the second time around. Something like an ordered
// Set, and a loop that: pops the first kref off, processes it (maybe
// adding more krefs), repeats until the thing is empty.
const actions = new Set();
for (const kref of maybeFreeKrefs.values()) {
const { type } = parseKernelSlot(kref);
if (type === 'promise') {
const kpid = kref;
const kp = getKernelPromise(kpid);
if (kp.refCount === 0) {
let idx = 0;
// TODO (#9889) don't assume promise is settled
for (const slot of kp.data.slots) {
// Note: the following decrement can result in an addition to the
// maybeFreeKrefs set, which we are in the midst of iterating.
Expand All @@ -1509,32 +1556,74 @@ export default function makeKernelKeeper(
deleteKernelPromise(kpid);
}
}

if (type === 'object') {
const { reachable, recognizable } = getObjectRefCount(kref);
if (reachable === 0) {
const ownerVatID = ownerOfKernelObject(kref);
if (ownerVatID) {
// We avoid ownerOfKernelObject(), which will report
// 'undefined' if the owner is dead (and being slowly
// deleted). Message delivery should use that, but not us.
const ownerKey = `${kref}.owner`;
let ownerVatID = kvStore.get(ownerKey);
const terminated = terminatedVats.includes(ownerVatID);

// Some objects that are still owned, but the owning vat
// might still alive, or might be terminated and in the
// process of being deleted. These two clauses are
// mutually exclusive.

if (ownerVatID && !terminated) {
const vatKeeper = provideVatKeeper(ownerVatID);
const isReachable = vatKeeper.getReachableFlag(kref);
if (isReachable) {
const vatConsidersReachable = vatKeeper.getReachableFlag(kref);
if (vatConsidersReachable) {
// the reachable count is zero, but the vat doesn't realize it
actions.add(`${ownerVatID} dropExport ${kref}`);
}
if (recognizable === 0) {
// TODO: rethink this
// assert.equal(isReachable, false, `${kref} is reachable but not recognizable`);
// TODO: rethink this assert
// assert.equal(vatConsidersReachable, false, `${kref} is reachable but not recognizable`);
actions.add(`${ownerVatID} retireExport ${kref}`);
}
} else if (recognizable === 0) {
// unreachable, unrecognizable, orphaned: delete the
// empty refcount here, since we can't send a GC
// action without an ownerVatID
deleteKernelObject(kref);
} else if (ownerVatID && terminated) {
// When we're slowly deleting a vat, and one of its
// exports becomes unreferenced, we obviously must not
// send dropExports or retireExports into the dead vat.
// We fast-forward the abandonment that slow-deletion
// would have done, then treat the object as orphaned.

const { vatSlot } = getReachableAndVatSlot(ownerVatID, kref);
// delete directly, not orphanKernelObject(), which
// would re-submit to maybeFreeKrefs
kvStore.delete(ownerKey);
kvStore.delete(`${ownerVatID}.c.${kref}`);
kvStore.delete(`${ownerVatID}.c.${vatSlot}`);
// now fall through to the orphaned case
ownerVatID = undefined;
}

// Now handle objects which were orphaned. NOTE: this
// includes objects which were owned by a terminated (but
// not fully deleted) vat, where `ownerVatID` was cleared
// in the last line of that previous clause (the
// fall-through case). Don't try to change this `if
// (!ownerVatID)` into an `else if`: the two clauses are
// *not* mutually-exclusive.

if (!ownerVatID) {
// orphaned and unreachable, so retire it. If the kref
// is recognizable, then we need retireKernelObjects()
// to scan for importers and send retireImports (and
// delete), else we can call deleteKernelObject directly
if (recognizable) {
retireKernelObjects([kref]);
} else {
deleteKernelObject(kref);
}
}
}
}
}
setGCActions(actions);
addGCActions([...actions]);
}
maybeFreeKrefs.clear();
}
Expand Down Expand Up @@ -1824,6 +1913,7 @@ export default function makeKernelKeeper(
kernelObjectExists,
getImporters,
orphanKernelObject,
retireKernelObjects,
deleteKernelObject,
pinObject,

Expand Down
3 changes: 0 additions & 3 deletions packages/SwingSet/src/kernel/vatTranslator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -481,9 +481,6 @@ function makeTranslateVatSyscallToKernelSyscall(vatID, kernelKeeper) {
assert.equal(allocatedByVat, true); // abandon *exports*, not imports
// kref must already be in the clist
const kref = mapVatSlotToKernelSlot(vref, gcSyscallMapOpts);
// note that this is effectful and also performed outside of a syscall
// by processUpgradeVat in {@link ./kernel.js}
vatKeeper.deleteCListEntry(kref, vref);
return kref;
});
kdebug(`syscall[${vatID}].abandonExports(${krefs.join(' ')})`);
Expand Down
Loading

0 comments on commit 7f12e80

Please sign in to comment.