Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(swingset): allow buildRootObject to return a Promise #5249

Closed
wants to merge 1 commit into from

Conversation

warner
Copy link
Member

@warner warner commented Apr 29, 2022

New (or upgraded) vats are defined by a user-provided
buildRootObject function. This function has two responsibilities:

  • provide a root object
  • use defineDurableKind to bind behavior to any pre-existing durable
    kinds

Previously, buildRootObject was given exactly one "turn" to fulfill
these responsibilities: it was required to return the root object
synchronously. If it returned a Promise, that was rejected as an
error.

This requirement is at odds with the needs for updated ZCF vats to
evaluate their contract code and interact with it (giving the contract
a chance to bind their own Kind behavior). The importBundle function
we use to load contracts is async. However, it is still "prompt": it
needs no IO, nor any messages leaving the vat. So it will still
complete within the crank, just not synchronously within the turn.

So this commit changes liveslots to allow buildRootObject to return
a Promise for the root object. The requirement, however, is that this
Promise must resolve by the end of the initial crank (before
waitUntilQuiescent / setImmediate determines that the promise
queue is empty). If the Promise is not fulfilled by then, the vat
creation/update is considered broken.

closes #5246

New (or upgraded) vats are defined by a user-provided
`buildRootObject` function. This function has two responsibilities:

* provide a root object
* use `defineDurableKind` to bind behavior to any pre-existing durable
  kinds

Previously, `buildRootObject` was given exactly one "turn" to fulfill
these responsibilities: it was required to return the root object
synchronously. If it returned a Promise, that was rejected as an
error.

This requirement is at odds with the needs for updated ZCF vats to
evaluate their contract code and interact with it (giving the contract
a chance to bind their own Kind behavior). The `importBundle` function
we use to load contracts is async. However, it is still "prompt": it
needs no IO, nor any messages leaving the vat. So it will still
complete within the *crank*, just not synchronously within the turn.

So this commit changes liveslots to allow `buildRootObject` to return
a Promise for the root object. The requirement, however, is that this
Promise must resolve by the end of the initial crank (before
`waitUntilQuiescent` / `setImmediate` determines that the promise
queue is empty). If the Promise is not fulfilled by then, the vat
creation/update is considered broken.

closes #5246
@warner warner added the SwingSet package: SwingSet label Apr 29, 2022
@warner warner self-assigned this Apr 29, 2022
@warner warner requested review from FUDCo and mhofman April 29, 2022 15:56
Copy link
Member

@mhofman mhofman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine by me, and I think it's a good new safety net in case the deliver to user code machinery in liveslots is updated in the future and introduces a stall. That's why I'd prefer the error message to be more generic.

return gcTools.waitUntilQuiescent().then(() => {
afterDispatchActions();
return p;
// eslint-disable-next-line prefer-promise-reject-errors
return complete ? p : Promise.reject('buildRootObject unresolved');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The not complete rejection is technically not startVat/buildRootObject specific, right? In practice it doesn't happen for other deliveries, but I'm a little concerned about using a specific call out here as it may be confusing to readers.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct.. the only dispatch that pays attention to a userspace-provided Promise is startVat, so the only place that can cause !complete is buildRootObject, so the error message is specialized. I'll add a comment to that effect.

@warner warner closed this Apr 29, 2022
@warner warner deleted the 5246-async-buildrootobject branch April 29, 2022 16:15
@warner
Copy link
Member Author

warner commented Apr 29, 2022

oops, I deleted the branch when I meant to be updating it

@warner
Copy link
Member Author

warner commented Apr 29, 2022

GitHub wouldn't let me re-open this PR, so I opened a new one: #5250

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

async buildRootObject
2 participants