Skip to content

Commit

Permalink
fix(liveslots): avoid slotToVal memory leak for watched promises
Browse files Browse the repository at this point in the history
Liveslots has a bug (#10757) which leaks slotToVal entries when a
tracked Promise is still being held in virtual data (e.g. a
merely-virtual MapStore) at the time it becomes settled. This is
triggered by `watchPromise` because of the order in which we attach
two handlers: one which notices the resolution and is inhibited from
deleting the slotToVal entry, and a second which removes the Promise
from the (virtual) `promiseRegistrations` collection (thus enabling
the deletion). For any watched Promise that is resolved, we leave a
`slotToVal` entry (with an empty WeakRef) in RAM until the end of the
incarnation.

This PR does not fix the underlying bug, but it rearranges the handler
order to avoid triggering it.

The attached unit test fails with the original handler
order (`slotToVal.size` grows), and passes with the swapped
order (`slotToVal.size` remains constant).

closes #10756
refs #10706
  • Loading branch information
warner committed Dec 20, 2024
1 parent 9cb1472 commit bec3f6e
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 3 deletions.
6 changes: 3 additions & 3 deletions packages/swingset-liveslots/src/watchedPromises.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,13 +235,13 @@ export function makeWatchedPromiseManager({
} else {
watchedPromiseTable.init(vpid, harden([[watcher, ...args]]));

promiseRegistrations.init(vpid, p);
pseudoThen(p, vpid);

// Ensure that this vat's promises are rejected at termination.
if (maybeExportPromise(vpid)) {
syscall.subscribe(vpid);
}

promiseRegistrations.init(vpid, p);
pseudoThen(p, vpid);
}
});
}
Expand Down
40 changes: 40 additions & 0 deletions packages/swingset-liveslots/test/watch-promise.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import test from 'ava';

import { Far } from '@endo/marshal';
import { makePromiseKit } from '@endo/promise-kit';
import { setupTestLiveslots } from './liveslots-helpers.js';

const build = vatPowers => {
const { VatData } = vatPowers;
const { makeKindHandle, defineDurableKind, watchPromise } = VatData;

const kh = makeKindHandle('handler');
const init = () => ({});
const behavior = {
onFulfilled: _value => 0,
onRejected: _reason => 0,
};
const makeHandler = defineDurableKind(kh, init, behavior);

return Far('root', {
async run() {
const pr = makePromiseKit();
const handler = makeHandler();
watchPromise(pr.promise, handler);
pr.resolve('ignored');
},
});
};

test('watched local promises should not leak slotToVal entries', async t => {
const { dispatchMessage, testHooks } = await setupTestLiveslots(
t,
build,
'vatA',
);
const { slotToVal } = testHooks;
const initial = slotToVal.size;

await dispatchMessage('run');
t.is(slotToVal.size, initial);
});

0 comments on commit bec3f6e

Please sign in to comment.