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 no-op syscall.dropImports to vat-kernel protocol #2635

Closed
warner opened this issue Mar 14, 2021 · 2 comments · Fixed by #2654
Closed

add no-op syscall.dropImports to vat-kernel protocol #2635

warner opened this issue Mar 14, 2021 · 2 comments · 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?

The first steps of the #2615 GC task includes the creation of a syscall.dropImports(vrefs), which accepts a list of vat references (e.g. o-12) that the vat has stopped using. Eventually this will be triggered by FinalizationRegistry callbacks at end-of-crank. Eventually the kernel will process the dropImports by decrementing refcounts and releasing other objects. But for now, merely having a no-op syscall will be sufficient to allow both of those future steps to be developed in parallel.

The vrefs will be limited to object IDs for now, I don't think we have a clear understanding of how we could drop unreferenced unresolved promises correctly, and the GC problem is less severe for Promises because most of them get resolved sooner or later, and we already have a working mechanism for retiring and releasing resolved promise IDs.

Description of the Design

syscall.dropImports([vrefs])

  • dropImports can be called at any time during the crank. Normal vats will only call it at end-of-crank (driven by liveslots checking the finalizers), but the comms vat will call it in response to many inbound comms.dropImport messages (Cope with asynchrony in remote retirement of promise IDs #2509 (comment)) that are freely interleaved with other syscalls.
  • All vrefs must be o-NN (imports), not o+NN (exports): we'll use a different mechanism for revoke a Remotable #2070 object revocation, because those will need an Error to go along with each
  • All vrefs must be previously imported: they must already be in the c-list. We may skip this check while the syscall is in the no-op stage, but eventually it will be a vat-fatal error to drop something that wasn't previously imported (or drop something twice without an intervening new import)
  • Once dropped, the vat must never reference the vref again, unless the kernel references it first (causing a re-import). In general we'll have the kernel allocate new vrefs and never trigger a re-import, but while the syscall is a no-op, the kernel won't be deleting the c-list entry and thus re-delivery of the same object will look exactly like a re-import of the same ID.
  • Once dropped, the vat is not expected to remember the vref: if the kernel does re-reference the ID, the vat is allowed (and expected) to create a brand new Presence to represent the new import.

Security Considerations

Test Plan

I intend to write unit tests to assert that this syscall can be made from all our vat runners. Since we decided the XS vat runner didn't need to support setup-based "raw vats", I need a way to reliably trigger dropImports from user-level vat code. When we get the vat-side finalizers (and explicit gc() invocation) working, this will be as simple as not retaining the imported Presence, but until that point we need some other way to trigger dropImports.

I propose to add a vatPowers.disavow(presence) function, disabled by default, that can trigger a dropImports. #2636
has the writeup of that proposal.

@warner
Copy link
Member Author

warner commented Mar 15, 2021

In my branch, syscall.dropImports is slightly more than a no-op: it deletes the c-list entry on the importing vat. This will call decrementRefCount() on the kernel object-id (the koid), but that function does not currently do anything in response to objects being decref-ed, only promises.

My vatTranslator function deletes the c-list entries while it is holding both vref and kref, and then delivers the set of krefs to the kernelSyscall dropImports function. My plan is for the latter to use the set of newly-dereferenced krefs as the starting point of a "did we decrement to zero?" check, to GC any non-cycles that just became dereferenced. But that might change as we figure out the refcount management part of the GC task.

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
Copy link
Member Author

warner commented Mar 17, 2021

Note that dropImports is currently recorded in the transcript. That will need to change when we have vats emitting it in reaction to finalizers, for the non-consensus mode (i.e. ag-solo). Once that becomes a possibility, we should not require that transcript replays emit the same dropImports syscalls as the original did, which means we should probably omit them from the transcript altogether.

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