Skip to content

Commit

Permalink
fix: Move upgrade-time abandonExports responsibility into the kernel
Browse files Browse the repository at this point in the history
Fixes #6696
  • Loading branch information
gibson042 committed Mar 23, 2023
1 parent 34c6345 commit 66ac657
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 19 deletions.
31 changes: 19 additions & 12 deletions packages/SwingSet/src/kernel/kernel.js
Original file line number Diff line number Diff line change
Expand Up @@ -885,25 +885,32 @@ export default function buildKernel(
return results;
}

// stopVat succeeded. finish cleanup on behalf of the worker.
// stopVat succeeded, now we finish cleanup on behalf of the worker

// TODO: send BOYD to the vat, to give it one last chance to clean
// up, drop imports, and delete durable data. If we ever have a
// vat that is so broken it can't do BOYD, we can make that
// optional. #7001
// TODO: send BOYD so the terminating vat has one last chance to clean
// up, drop imports, and delete durable data.
// If a vat is so broken it can't do BOYD, we can make that optional.
// https://github.com/Agoric/agoric-sdk/issues/7001

// walk c-list for all decided promises, reject them all
// reject all promises for which the vat was decider
for (const kpid of kernelKeeper.enumeratePromisesByDecider(vatID)) {
resolveToError(kpid, disconnectionCapData, vatID);
}

// TODO: getNonDurableObjectExports, synthesize abandonVSO,
// execute it as if it were a syscall. (maybe distinguish between
// reachable/recognizable exports, abandon the reachable, retire
// the recognizable) #6696
// simulate an abandonExports syscall from the vat,
// without making an *actual* syscall that could pollute logs
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} */
kernelKeeper.orphanKernelObject(kref, vatID);
}

// cleanup done, now we stop the worker, delete the transcript and
// any snapshot
// cleanup done, now we reset the worker to a clean state with no
// transcript or snapshot and prime everything for the next incarnation.

await vatWarehouse.resetWorker(vatID);
const source = { bundleID };
Expand Down
2 changes: 2 additions & 0 deletions packages/SwingSet/src/kernel/kernelSyscall.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,8 @@ export function makeKernelSyscallHandler(tools) {
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
32 changes: 32 additions & 0 deletions packages/SwingSet/src/kernel/state/kernelKeeper.js
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,37 @@ export default function makeKernelKeeper(kernelStorage, kernelSlog) {
kvStore.set(`${kernelSlot}.refCount`, `${reachable},${recognizable}`);
}

/**
* Iterate over non-durable objects exported by a vat.
*
* @param {string} vatID
* @yields {{kref: string, vref: string}}
*/
function* enumerateNonDurableObjectExports(vatID) {
insistVatID(vatID);
// vrefs for exported objects start with o+NN (ephemeral),
// o+vNN/MM (merely-virtual), or o+dNN/MM (durable).
// We iterate through all ephemeral and virtual entries so the kernel
// can ensure that they are abandoned by a vat being upgraded.
const prefix = `${vatID}.c.`;
const ephStart = `${prefix}o+`;
const durStart = `${prefix}o+d`;
const virStart = `${prefix}o+v`;
/** @type {[string, string?][]} */
const ranges = [[ephStart, durStart], [virStart]];
for (const range of ranges) {
for (const k of enumeratePrefixedKeys(kvStore, ...range)) {
const vref = k.slice(prefix.length);
// exclude the root object, which is replaced by upgrade
if (vref !== 'o+0') {
const kref = kvStore.get(k);
assert.typeof(kref, 'string');
yield { kref, vref };
}
}
}
}

/**
* Allocate a new koid.
*
Expand Down Expand Up @@ -1574,6 +1605,7 @@ export default function makeKernelKeeper(kernelStorage, kernelSlog) {
incrementRefCount,
decrementRefCount,
getObjectRefCount,
enumerateNonDurableObjectExports,

addToRunQueue,
isRunQueueEmpty,
Expand Down
17 changes: 14 additions & 3 deletions packages/SwingSet/src/kernel/state/storageHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,26 @@

import { assert } from '@agoric/assert';

export function* enumeratePrefixedKeys(kvStore, prefix) {
// return an iterator of all existing keys that start with
// ${prefix}, in lexicographic order, excluding ${prefix} itself
/**
* Iterate over keys with a given prefix, in lexicographic order,
* excluding an exact match of the prefix.
*
* @param {KVStore} kvStore
* @param {string} prefix
* @param {string} [exclusiveEnd]
* @yields {string} the next key with the prefix that is not >= exclusiveEnd
*/
export function* enumeratePrefixedKeys(kvStore, prefix, exclusiveEnd) {
/** @type {string | undefined} */
let key = prefix;
for (;;) {
key = kvStore.getNextKey(key);
if (!key || !key.startsWith(prefix)) {
break;
}
if (exclusiveEnd && key >= exclusiveEnd) {
break;
}
yield key;
}
}
Expand Down
2 changes: 2 additions & 0 deletions packages/SwingSet/src/kernel/vatTranslator.js
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,8 @@ 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;
});
Expand Down
15 changes: 15 additions & 0 deletions packages/SwingSet/test/test-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,21 @@ test('storage helpers', t => {
'bar.3',
'bar.5',
]);

t.deepEqual(Array.from(enumeratePrefixedKeys(kv, 'bar', 'bar.1')), []);
t.deepEqual(Array.from(enumeratePrefixedKeys(kv, 'bar', 'bar.4')), [
'bar.1',
'bar.3',
]);
t.deepEqual(Array.from(enumeratePrefixedKeys(kv, 'bar', 'bar.5')), [
'bar.1',
'bar.3',
]);
t.deepEqual(Array.from(enumeratePrefixedKeys(kv, 'bar', 'bar.6')), [
'bar.1',
'bar.3',
'bar.5',
]);
});

function buildKeeperStorageInMemory() {
Expand Down
16 changes: 12 additions & 4 deletions packages/swingset-liveslots/src/stop-vat.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { enumerateKeysWithPrefix } from './vatstore-iterators.js';

const rootSlot = makeVatSlot('object', true, 0n);

// eslint-disable-next-line no-unused-vars
function identifyExportedRemotables(
vrefSet,
{ exportedRemotables, valToSlot },
Expand All @@ -59,6 +60,7 @@ function identifyExportedRemotables(
}
}

// eslint-disable-next-line no-unused-vars
function identifyExportedFacets(vrefSet, { syscall, vrm }) {
// Find all exported (non-durable) virtual object facets, which are
// doomed because merely-virtual objects don't survive upgrade. We
Expand Down Expand Up @@ -96,6 +98,7 @@ function identifyExportedFacets(vrefSet, { syscall, vrm }) {
}
}

// eslint-disable-next-line no-unused-vars
function abandonExports(vrefSet, tools) {
// Pretend the kernel dropped everything in the set. The Remotables
// will be removed from exportedRemotables. If the export was the
Expand Down Expand Up @@ -285,10 +288,15 @@ export async function releaseOldState(tools) {
// refcount decrements which may drop some virtuals from the DB. It
// might also drop some objects from RAM.

const abandonedVrefSet = new Set();
identifyExportedRemotables(abandonedVrefSet, tools);
identifyExportedFacets(abandonedVrefSet, tools);
abandonExports(abandonedVrefSet, tools);
// TODO: Decide how much (if any) cleanup to do here in the vat.
// The kernel simulates abandonExports as part of vat upgrade,
// but does not decrement vat-side refcounts which could allow us to
// drop durable objects that were only being kept alive by references from
// non-durable objects.
// const abandonedVrefSet = new Set();
// identifyExportedRemotables(abandonedVrefSet, tools);
// identifyExportedFacets(abandonedVrefSet, tools);
// abandonExports(abandonedVrefSet, tools);

// bringOutYourDead remains to ensure that the LRU cache is flushed,
// but the rest of this function has been disabled to improve stop-vat
Expand Down

0 comments on commit 66ac657

Please sign in to comment.