fix(liveslots): avoid slotToVal memory leak for watched promises (#10758

)

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

packages/swingset-liveslots/src/watchedPromises.js

@@ -235,13 +235,23 @@ export function makeWatchedPromiseManager({
       } else {
         watchedPromiseTable.init(vpid, harden([[watcher, ...args]]));
 
+        promiseRegistrations.init(vpid, p);
+
+        // pseudoThen registers a settlement callback that will remove
+        // this promise from promiseRegistrations and
+        // watchedPromiseTable. To avoid triggering
+        // https://github.com/Agoric/agoric-sdk/issues/10757 and
+        // preventing slotToVal cleanup, the `pseudoThen()` should
+        // precede `maybeExportPromise()`. This isn't foolproof, but
+        // does mitigate in advance of a proper fix. See #10756 for
+        // details of this particular mitigation, and #10757 for the
+        // deeper bug.
+        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);
       }
     });
   }

packages/swingset-liveslots/test/watch-promise.test.js

@@ -0,0 +1,42 @@
+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);
+  await dispatchMessage('run');
+  t.is(slotToVal.size, initial);
+});