match incref/decref calls within kernel

[warner]

While working through unit tests for #3109, I noticed that we weren't consistently performing matched incref/decref calls in all places within the kernel.

If we trace a typical message, it begines life in the kernel-side handler for syscall.send, which routes to kernelSyscall.js: doSend():

• slots are increfed in doSend() before the message is passed to kernelKeeper.addToRunQueue
• later, when the send event is pulled off the run-queue by kk.getNextMsg, the slots are not decrefed at that time
• the event is classified by processQueueMessage
  • for send events, the slots are decrefed in processQueueMessage before being sent to deliverToTarget
  • create-vat events do not have any slots, so nothing needs to be done for them
  • notify events name a promise, however I don't think we need to hold a reference to it (and in fact we probably shouldn't, since it might have been resolved and delivered already, and thus we might have been able to delete it already)
  • GC actions have slots but should specifically not cause refcounts to be retained
• deliverToTarget breaks the target down into several cases:
  • targets which are objects, or promises resolved to a single object, route to deliverToVat
  • targets which are orphaned objects (exports of a terminated vat) drop the message and provoke resolveToError on their result promise, if any
  • targets which are rejected promises provoke resolveToError
  • targets which are resolved to non-objects provoke resolveToError
  • targets which are unresolved promises whose decider enabled pipelining use deliverToVat
  • unresolved promises whose decider did not enable pipelining, or without a decider, use kk.addMessageToPromiseQueue to move the message to the kernel promise table's queue entry
• when a promise is resolved in kernel.js: doResolve, kk.getResolveablePromise returns the complete state of the promise (including the queued messages)
  • we incref the resolution data's slots, then call kk.resolveKernelPromise is used to replace the saved state with the resolution data
  • then kernel.js: notifySubscribersAndQueue is called to push a notify event for each subscriber, and call kernelSyscallHandler.send() for each queued message
    • this delegates to doSend, so it increfs the message slots just as if a vat did a brand new syscall.send
• when a resolved promise becomes unreferenced, it is deleted with a call to kernelKeeper.js: deleteKernelPromise() from within processRefcounts(), which uses deleteKernelPromiseState() to delete all the relevant keys. This does not change the refcounts of the resolution data's slots.

The first mismatch I'm looking at is a message which gets queued on an unresolved promise. We decref its slots when pulling it off the run-queue, but do not increment them when adding to the promise queue. We do not decref them when removing from the promise queue, but we do incref them when adding the message back to the run-queue. For promises that get resolved, the final refcount is correct, but they'll be too low while the message is on the promise queue, and are thus vulnerable to being dropped early.

The second mismatch is in the promise's resolution data. This is incremented during doResolve, and decremented in processRefcounts just before calling deleteKernelPromise. I think the incref/decrefs are matched and sound, but it feels dangerous to have the two calls happen in different places: I kind of want both increment and decrement to happen in kernelKeeper.js.

[warner]

cc @FUDCo

[warner]

For messages, here's one approach:

• decref the slots when we pull the event off the run-queue (in kernelKeeper.getNextMsg)
• incref them when adding it to either the run-queue (in kernelKeeper.addToRunQueue) or a promise-queue (in kernelKeeper.addMessageToPromiseQueue)
• decref them when removing all messages from the promise queue (currently done in kernel.js: notifySubscribersAndQueue, but maybe we should add a kernelKeeper.drainMessagesFromPromiseQueue iterator that also decrefs)
• incref them when adding back to the run-queue just like we currently do in notifySubscribersAndQueue / kernelSyscallHandler.send

I like the idea of strictly managing the refcounts from within kernelKeeper.js, that feels far more likely to remain correct under maintenance.

One downside is the DB churn when we pull an item off the run-queue only to push it onto a promise-queue, and likewise when promise-queue messages are transferred right back to the run-queue. If/when we implement promise forwarding (resolving a promise to a different promise), we might do more of the same, depending upon the implementation and the ordering properties we choose to provide.

So an alternative approach would avoid the churn when moving messages, and instead aim to incref/decref only upon the creation or destruction of messages:

• incref during syscall.send
• decref in deliverToTarget, but only if we call deliverToVat or resolveToError
  • if we call kk.addMessageToPromiseQueue, leave the refcounts alone
  • if we recurse back into deliverToTarget, delegate the decision to the recursive call
• notifySubscribersAndQueue must not do a new incref, so it must join the control flow of syscall.send/kernelSyscallHandler.send/doSend somewhere after the incref it does

This would result in less down-then-up refcount churn, but would distribute the incref/decref calls out through more code, which feels less reliable to me.

[warner]

@FUDCo had a great idea: add a method to kernelKeeper that moves all the promise-queued messages to the run-queue, leaving their refcounts intact. That will reduce half the churn.