diff --git a/packages/SwingSet/src/kernel/kernel.js b/packages/SwingSet/src/kernel/kernel.js index 01b4d54979c..1d9169258d8 100644 --- a/packages/SwingSet/src/kernel/kernel.js +++ b/packages/SwingSet/src/kernel/kernel.js @@ -381,7 +381,6 @@ export default function buildKernel( terminationTrigger = undefined; postAbortActions = { meterDeductions: [], // list of { meterID, compute } - meterNotifications: [], // list of meterID }; } resetDeliveryTriggers(); @@ -397,17 +396,19 @@ export default function buildKernel( function deductMeter(meterID, compute, firstTime) { assert.typeof(compute, 'bigint'); const res = kernelKeeper.deductMeter(meterID, compute); - // we also recode deduction and any notification in postAbortActions, - // which are executed if the delivery is being rewound for any reason - // (syscall error, res.underflow), so their side-effects survive + + // We record the deductMeter() in postAbortActions.meterDeductions. If + // the delivery is rewound for any reason (syscall error, res.underflow), + // then deliverAndLogToVat will repeat the deductMeter (which will repeat + // the notifyMeterThreshold), so their side-effects will survive the + // abortCrank(). But we don't record it (again) during the repeat, to + // make sure exactly one copy of the changes will be committed. + if (firstTime) { postAbortActions.meterDeductions.push({ meterID, compute }); } if (res.notify) { notifyMeterThreshold(meterID); - if (firstTime) { - postAbortActions.meterNotifications.push(meterID); - } } return res.underflow; } @@ -730,14 +731,11 @@ export default function buildKernel( // errors unwind any changes the vat made abortCrank(); didAbort = true; - // but metering deductions or underflow notifications must survive - const { meterDeductions, meterNotifications } = postAbortActions; + // but metering deductions and underflow notifications must survive + const { meterDeductions } = postAbortActions; for (const { meterID, compute } of meterDeductions) { deductMeter(meterID, compute, false); - } - for (const meterID of meterNotifications) { - // reads meter.remaining, so must happen after deductMeter - notifyMeterThreshold(meterID); + // that will re-push any notifications } } // state changes reflecting the termination must also survive, so diff --git a/packages/SwingSet/test/metering/test-dynamic-vat-metered.js b/packages/SwingSet/test/metering/test-dynamic-vat-metered.js index 1886e899455..09bc1304444 100644 --- a/packages/SwingSet/test/metering/test-dynamic-vat-metered.js +++ b/packages/SwingSet/test/metering/test-dynamic-vat-metered.js @@ -1,4 +1,5 @@ /* global __dirname */ +/* eslint-disable no-await-in-loop */ // eslint-disable-next-line import/order import { test } from '../../tools/prepare-test-env-ava.js'; @@ -374,4 +375,98 @@ test('unlimited meter', async t => { kpidRejected(t, c, doneKPID, 'Compute meter exceeded'); }); -// TODO notify and underflow in the same delivery, to exercise postAbortActions +// Cause both a notify and an underflow in the same delivery. Without +// postAbortActions, the notify would get unwound by the vat termination, and +// would never be delivered. +test('notify and underflow', async t => { + const managerType = 'xs-worker'; + const { kernelBundles, dynamicVatBundle, bootstrapBundle } = t.context.data; + const config = { + bootstrap: 'bootstrap', + vats: { + bootstrap: { + bundle: bootstrapBundle, + }, + }, + }; + const hostStorage = provideHostStorage(); + const c = await buildVatController(config, [], { + hostStorage, + kernelBundles, + }); + c.pinVatRoot('bootstrap'); + + // let the vatAdminService get wired up before we create any new vats + await c.run(); + + // create a meter with 200k remaining and a notification threshold of 1 + const cmargs = capargs([200000n, 1n]); // remaining, notifyThreshold + const kp1 = c.queueToVatRoot('bootstrap', 'createMeter', cmargs); + await c.run(); + const { marg, meterKref } = extractSlot(t, c.kpResolution(kp1)); + // and watch for its notifyThreshold to fire + const notifyKPID = c.queueToVatRoot( + 'bootstrap', + 'whenMeterNotifiesNext', + capargs([marg], [meterKref]), + ); + + // 'createVat' will import the bundle + const cvargs = capargs( + [dynamicVatBundle, { managerType, meter: marg }], + [meterKref], + ); + const kp2 = c.queueToVatRoot('bootstrap', 'createVat', cvargs); + await c.run(); + const res2 = c.kpResolution(kp2); + t.is(JSON.parse(res2.body)[0], 'created', res2.body); + const doneKPID = res2.slots[0]; + + async function getMeter() { + const args = capargs([marg], [meterKref]); + const kp = c.queueToVatRoot('bootstrap', 'getMeter', args); + await c.run(); + const res = c.kpResolution(kp); + const { remaining } = parse(res.body); + return remaining; + } + + async function consume(shouldComplete) { + const kp = c.queueToVatRoot('bootstrap', 'run', capargs([])); + await c.run(); + if (shouldComplete) { + t.is(c.kpStatus(kp), 'fulfilled'); + t.deepEqual(c.kpResolution(kp), capargs(42)); + } else { + t.is(c.kpStatus(kp), 'rejected'); + kpidRejected(t, c, kp, 'vat terminated'); + } + } + + // run three consume() calls to measure the usage of the last + await consume(true); + await consume(true); + const remaining1 = await getMeter(); + await consume(true); + let remaining = await getMeter(); + const oneCycle = remaining1 - remaining; + // console.log(`one cycle appears to use ${oneCycle} computrons`); + + // keep consuming until there is less than oneCycle remaining + while (remaining > oneCycle) { + await consume(true); + remaining = await getMeter(); + // console.log(` now ${remaining}`); + } + + // the next cycle should underflow *and* trip the absurdly low notification + // threshold + // console.log(`-- doing last consume()`); + await consume(false); + remaining = await getMeter(); + t.is(remaining, 0n); // this checks postAbortActions.deductMeter + t.is(c.kpStatus(notifyKPID), 'fulfilled'); // and pAA.meterNotifications + const notification = c.kpResolution(notifyKPID); + t.is(parse(notification.body).value, 0n); + kpidRejected(t, c, doneKPID, 'meter underflow, vat terminated'); +});