XS isn't garbage-collecting objects when expected

[warner]

We're seeing unexpected behavior from the XS garbage collector. It seems to be retaining objects used in eventual-sends, where the V8 collector lets them get collected.

I'm still characterizing this, but a simple example is the following vat:

export function buildRootObject() {
  return Far('root', {
    two(A, B) { // A=ko26 B=ko27
      return E(A).hello(B);
    },
  });
}

(A and B are simple Remotables exported by some other vat, with a dummy hello: () => {} method)

This results in two cranks on our vat: the first delivers two(A,B), and the second notifies the vat about the hello() result promise being resolved.

At the end of each crank, our liveSlots.js layer is waiting for the vat's code to become idle (with await new Promise(setImmediate)), then forcing a GC sweep, with several additional setImmediate stalls introduced that were experimentally determined to allow finalizers to run. When the finalizers run, they accumulate the vrefs in a set, and then perform syscall.dropImports().

When we run this under Node.js, the two objects are collected during the second crank, causing our vat to do a syscall.dropImports() for the vrefs that represent both A and B. When we run this under XS, neither object is collected (i.e. these finalizers were not called by the end of the second crank). Since no further deliveries are made to this vat, there are no further opportunities for the vat to inform the kernel about the objects being dropped.

The same happens if we replace the return with async/await:

export function buildRootObject() {
  return Far('root', {
    async two(A, B) { // A=ko26 B=ko27
      await E(A).hello(B);
    },
  });
}

But if we remove the return:

export function buildRootObject() {
  return Far('root', {
    two(A, B) { // A=ko26 B=ko27
      E(A).hello(B);
    },
  });
}

Then we still get the same two cranks (however nobody is watching the result promise, so no user-level code runs when the notify arrives). Node.js collects both A and B during the second crank as before. Under XS, A is collected at the end of the first crank, and B is never collected.

Ideally, both A and B would be collected at the end of the first crank: there are no references to either after the hello() message is sent.

Our standard coding style is to use async methods and await (although only at the top of the method scope, not inside loops or conditionals). So we're going to hit this case all of the time, and we need those objects to get released.

[warner]

I tried to build up a reduced test case "from below", by starting with a simple xsnap runner, and adding code from swingset/eventual-send until it started to exhibit the failure. I wasn't able to make it fail.

I've started to try "from above", by removing things from swingset until the failure goes away. The one datapoint I have so far, which maybe will mean something to @michaelfig or @erights , is that removing the returned Promise p at the end of liveslots' queueMessage() allows both A and B to be colected in the first crank.

agoric-sdk/packages/SwingSet/src/kernel/liveSlots.js

Line 649 in 5b38615

return p;

[warner]

The amazing Moddable folks identified the problem and provided a fix. In Patrick's words:

"The objects to be collected were referenced by the closures of a function, deeply nested within the reject handler of a promise. The promise was fulfilled and stored into your “pendingPromises” set. The issue seemed to be that promises retained references to handlers even when resolved. I fixed that. "

[warner]

PR #3416 landed the fix for this (in commit 8eeec28). I still need to run a perf test and count the drops, hopefully there will be more of them.

[warner]

I have an example of this, however it's not XS-specific (V8 fails to drop the object too). It's basically just:

    makeInvitationTarget(zoe) {
      return E(zoe).makeInvitationZoe();
    },

(where zoe is a Presence pointing to some other vat)

The invitation object passes through liveslots (and a HandledPromise or two), but the user-level code never sees it. The vat imports the invitation from zoe, resolves the result promise for makeInvitationTarget, and is then supposed to drop it. But it doesn't drop it.

I tried commenting out the knownResolutions table update (that's a WeakMap from Promise to the resolution data.. if the result promise were somehow being kept alive, that would have also kept the invitation alive), but that didn't make the problem go away.

I'm currently suspecting some odd cycle in HandledPromise and/or the way that liveslots uses it.

[warner]

I've got a failing test in branch 3406-xs-not-gc. With some extra debug prints, I see the two deliveries in the vat-target which is incorrectly holding on to the just-passing-through Presence:

calling makeInvitation
 ls-v2 ==== deliver [o+0].makeInvitationTarget
 ls-v2 +hpid0 (makeImportedPresence)
 ls-v2 adding o-50
 ls-v2  hpid0 res/rej
 ls-v2 +hpid1 (makeImportedPromise)
 ls-v2 processDeadSet ---
 ls-v2 ==== notify p+5
 ls-v2 notifyOnePromise p+5
 ls-v2 +hpid2 (makeImportedPresence)
 ls-v2 adding o-51
 ls-v2 notifyOnePromise did unser p+5
 ls-v2 pRec.resolve p+5 Alleged: Zoe Invitation payment <[Object: null prototype] [Alleged: Zoe Invitation payment] {}> [Alleged: Zoe Invitation payment] {}
 ls-v2  hpid2 res/rej
 ls-v2  hpid1 res/rej
 ls-v2 knownResolutions.set p-60 Alleged: Zoe Invitation payment <[Object: null prototype] [Alleged: Zoe Invitation payment] {}> [Alleged: Zoe Invitation payment] {}
 ls-v2 -hpid0
 ls-v2 processDeadSet ---
 ls-v2 finalize o-50

The +hpid lines show the creation of each HandledPromise. In the first crank (the dispatch.deliver for makeInvitationTarget), it creates the first HP (hpid0) to use for the zoe argument, which gets resolved immediately. That HP itself is not released until the end of the second delivery, which seems longer than expected but still good enough. Then the second HP (hpid1) is created to hold the result promise of the outgoing syscall.send('makeInvitationZoe').

In the second crank (the notify which resolves that result promise), we build a third HP (hpid2) and resolve it promptly to build the invitation Presence. We resolve hpid0, which causes a syscall.resolve() to pass the Invitation back to the caller of makeInvitationTarget.

Neither hpid1 nor hpid2 are getting finalized at any point. This might be the root cause, if the resolution value of hpid2 (the invitation is being held too.

[warner]

Big thanks to @michaelfig , we walked through this and found a problem in liveslots. Details will be in #3482. I'll leave this ticket open for an actual XS-specific problems we find once #3482 is fixed.

[dckc]

Big thanks to @michaelfig , we walked through this and found a problem in liveslots. Details will be in #3482. I'll leave this ticket open for an actual XS-specific problems we find once #3482 is fixed.

Did you find any XS-specific problems?

[warner]

#3488 is about a leak we found in the "vault" loadgen task, however we don't know if it's XS-specific or not. We decided to fix an API usage issue in the treasury contract first, then reevaluate to see if the leak is still present.

I'm going to close this ticket, and open a new one if/when we find a new leak.