Skip to content

Commit

Permalink
fix: rewrite liveslots use of HandledPromise, remove deliver() stall
Browse files Browse the repository at this point in the history
The exception-catching idiom I used in `deliver()` was introducing an
unnecessary stall into the result resolution pathway, making
`syscall.fulfill*` calls show up later than I expected. I rewrote it with a
different pattern (try/catch instead of `Promise.resolve().then()`), which
makes it easier to follow what we do and do not wait for. The new `deliver()`
does not return a Promise to the kernel.

The `makeQueued` function in liveslots was perhaps a bit overloaded, so I
refactored it into two distinct variants: `makeImportedPresence()` and
`makeImportedPromise()`. Each sets up a specific handler (`fulfilledHandler`
for presences, `unfulfilledHandler` for promises). This enabled
`convertSlotToVal` and `queueMessage` to be simplified. I added some
guard-against-confusion checks that can be removed once we're confident that
I'm calling HandledPromise correctly (and it's upholding its contract), which
should simplify the code further.

`thenResolve` and `thenReject` create callback functions which react to an
exported (or result) Promise being resolved, by telling the kernel about the
resolution (with some flavor of `syscall.fulfill*`). It is possible (albeit
kinda weird) for a vat to both subscribe to a promise *and* wind up with
decision making authority over it. I changed `thenResolve`/`thenReject` to
notice this situation, and locally resolve the Promise as if the kernel had
notified us about the resolution. For now, this is redundant (the local
resolution happens first, then some number of cranks later the kernel
delivers a duplicate resolution) and harmless. But once the retire-vpid
branch lands, the kernel will stop telling us about our own resolutions, and
this local path will be the only way the local Promise gets resolved.

I expanded test-vpid-liveslots to exercise resolution to a local object, in
addition to the other kinds of resolution. With the removal of the
unnecessary `deliver()` stall, the syscall traces no longer depend upon how
many stalls we did after the resolve, so the test no longer needs a
complicated set of options that switch on the stall count. Resolutions and
message sends now occur in the same order they appear in the vat code.

test-vpid-liveslots also exercises resolution and rejection to data which
contains a promise. We expect this to behave differently during the
retire-vpid work: during the initial phases of that task, we won't retire
vpids that were resolved to data which contains promises, to avoid endless
recursion. Later, after more careful analysis, we'll retire VPIDs in more
cases.

One test in test/message-patterns.js now pipelines better than it did before,
so I updated the expected log sequence.
  • Loading branch information
warner committed May 7, 2020
1 parent 2b931fc commit 42c2193
Show file tree
Hide file tree
Showing 3 changed files with 490 additions and 280 deletions.
Loading

0 comments on commit 42c2193

Please sign in to comment.