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

add vatPowers.disavow(presence) #2636

Closed
warner opened this issue Mar 14, 2021 · 1 comment · Fixed by #2654
Closed

add vatPowers.disavow(presence) #2636

warner opened this issue Mar 14, 2021 · 1 comment · Fixed by #2654
Assignees
Labels
enhancement New feature or request SwingSet package: SwingSet

Comments

@warner
Copy link
Member

warner commented Mar 14, 2021

What is the Problem Being Solved?

In #2635 I'll be adding a syscall.dropImports([vrefs]) feature, initially a no-op, to enable more of the #2615 GC work to proceed in parallel. To test this feature across all vat runners (including XS, which doesn't support "raw" setup()-based vats), I need a way for normal vat code to reliably trigger it.

Description of the Design

I propose to add a vatPowers.disavow(presence) function. This would only appear in vatPowers if the vat was created with vatOptions.enableDisavow = true, otherwise the function is omitted, and vats have no more authority than they did before.

Calling disavow(presence) causes the following:

  • Liveslots invokes syscall.dropImports([vref]) for the Presence's object ID.
  • The Presence's associated HandledPromise is told to reject any attempted message-sends with an error like Presence was disavowed
  • Any attempt to serialize the Presence (in the arguments of a message or resolution) is rejected with an error.
    • A message-send that uses the Presence in an argument will have its result promise rejected with an error.
    • A resolution that includes the Presence in the resolution value will fail, however there is no good way to signal the error, so the resolution will not happen, and an "unhandled rejection" will be logged, if we're logging those somehow.

(If this facility is not removed after we finish the GC task, a likely improvement would be to let disavow accept a non-default Error too, so real users could provide a hint as to why the Presence was disavowed).

The mechanism I have in mind is for disavow to do the following:

  • Look up the vref in valToSlot, throw error if not present.
  • Delete the valToSlot and slotToVal entries.
  • Record the presence -> error mapping in a new WeakMap, probably called disavowedPresences
  • In the fulfilledHandler created during makeImportedPresence, use the o argument to look up the Presence in disavowedPresences, if present, throw the error instead of calling queueMessage
  • In convertValToSlot, after checking valToSlot, and before calling exportPassByPresence, we check disavowedPresences, and throw the error if present.

The primary purpose of this feature is for testing, but I'm wondering if it might be useful for something else long-term. It's sort of a complement to the object revocation feature (#2070), except on imports instead of exports. It has the same non-ocap-ish authority problem, except worse, because at least for object revocation there's a specific point at which the object was created (when we invoke Far), which gives us a place to return a revocation authority that's distinct from the reference/invocation authority you get by receiving the Far/Remotable/Presence. To disavow an incoming Presence, there's no obvious place where one party has any better claim than anyone else. So for lack of any better place to put it, vatPowers seems like the least-wrong location (and it's disabled by default, the creator of the vat must opt the vat into having this thing).

@erights I'd love to know how much this makes you cringe :) . If we decided it was too horrible to contemplate, I could test syscall.dropImports with a raw setup-style vat, at the cost of not having test coverage on the XS runner (and maybe manually hack something together as a one-off to get it working there). Once we get to the later stage of GC in which finalizers are working, syscall.dropImports will be exercised by the GC tests. So we could either implement disavow now and then remove it once the finalizers are usable, or not implement disavow and survive with limited test coverage until that same point.

cc @dtribble

@warner warner added enhancement New feature or request SwingSet package: SwingSet labels Mar 14, 2021
@warner warner self-assigned this Mar 14, 2021
@warner
Copy link
Member Author

warner commented Mar 15, 2021

I spoke with @erights who didn't recoil in horror, but also wanted to make sure this didn't become an attractice nuisance. So I'm going to make the following changes:

  • remove enableDisavow from the list of options that createDynamicVat takes, so disavow will only be available to static vats (and even then only when the kernel config enables it)
  • change the error checks (when targeting or referencing a disavowed object) to kill the vat, using syscall.exit and an Error, rather than merely rejecting the specific message send that referenced the object
    • this also (violently) surfaces the previously silent-ish error when a promise resolution references the disavowed object in the resolution data

In general I'm going to consider disavow to be in the "necessary evil" category, where I'll get rid of it if possible. The reason to keep it around is to build a more satisfying test of this piece of the GC system. We'll certainly need to have a test that relies upon the behavior of an actual FinalizationRegistry, but I'd prefer to have that test be as small as possible, and test as much of the overall system as we can without using FR or WeakRef. And I suspect we'll continue to need disavow to achieve that.

warner added a commit that referenced this issue Mar 16, 2021
The new `vatPowers.disavow` doesn't do anything yet, but is at least
exercised by test-liveslots.js

The `enableDisavow` option is only available for static vats. The `endow`
function cannot be enabled on dynamic vats.

refs #2636, but won't close it until #2635 is implemented and `disavow()`
routes into `syscall.dropImports()`
warner added a commit that referenced this issue Mar 16, 2021
Vat code can now use `vatPowers.disavow(presence)` (if enabled for that vat),
which will invoke `syscall.dropImports`. The kernel will delete the entry
from the vat's c-list, however no further reference-count management will
occur (that is scheduled for #2646).

This should be enough to allow work to proceed on liveslots (using WeakRef and
FinalizationRegistry) in parallel with kernel-side improvements.

Note that referencing a disavowed object is vat-fatal, either as the target
of a message, the argument of a message, or the resolution of a promise.

closes #2635
closes #2636
warner added a commit that referenced this issue Mar 17, 2021
The new `vatPowers.disavow` doesn't do anything yet, but is at least
exercised by test-liveslots.js

The `enableDisavow` option is only available for static vats. The `endow`
function cannot be enabled on dynamic vats.

refs #2636, but won't close it until #2635 is implemented and `disavow()`
routes into `syscall.dropImports()`
warner added a commit that referenced this issue Mar 17, 2021
Vat code can now use `vatPowers.disavow(presence)` (if enabled for that vat),
which will invoke `syscall.dropImports`. The kernel will delete the entry
from the vat's c-list, however no further reference-count management will
occur (that is scheduled for #2646).

This should be enough to allow work to proceed on liveslots (using WeakRef and
FinalizationRegistry) in parallel with kernel-side improvements.

Note that referencing a disavowed object is vat-fatal, either as the target
of a message, the argument of a message, or the resolution of a promise.

closes #2635
closes #2636
warner added a commit that referenced this issue Mar 19, 2021
The new `vatPowers.disavow` doesn't do anything yet, but is at least
exercised by test-liveslots.js

The `enableDisavow` option is only available for static vats. The `endow`
function cannot be enabled on dynamic vats.

refs #2636, but won't close it until #2635 is implemented and `disavow()`
routes into `syscall.dropImports()`
warner added a commit that referenced this issue Mar 19, 2021
Vat code can now use `vatPowers.disavow(presence)` (if enabled for that vat),
which will invoke `syscall.dropImports`. The kernel will delete the entry
from the vat's c-list, however no further reference-count management will
occur (that is scheduled for #2646).

This should be enough to allow work to proceed on liveslots (using WeakRef and
FinalizationRegistry) in parallel with kernel-side improvements.

Note that referencing a disavowed object is vat-fatal, either as the target
of a message, the argument of a message, or the resolution of a promise.

closes #2635
closes #2636
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant