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

can virtual objects or the WeakStore hold a Promise? #2485

Closed
warner opened this issue Feb 19, 2021 · 11 comments
Closed

can virtual objects or the WeakStore hold a Promise? #2485

warner opened this issue Feb 19, 2021 · 11 comments
Assignees
Labels
enhancement New feature or request SwingSet package: SwingSet
Milestone

Comments

@warner
Copy link
Member

warner commented Feb 19, 2021

What is the Problem Being Solved?

I suspect that putting a Promise into the state of a virtual object will cause problems, as would storing one in the value of a WeakStore when the key is a virtual object. We should make sure this either works as expected, or is rejected clearly.

We'll serialize the Promise when the state.propname = setter is invoked, or when the init/set method on the WeakStore is run. Our serialization code thinks it is only used to send things into the kernel, so it will attach a .then, and when that fires, liveslots will do a syscall.resolve into the kernel. However in this case, the kernel has not yet been told about the Promise, so the syscall will probably fail (it's a spurious/unwanted resolution).

The slotToVal table will hold on to the actual Promise object until it gets resolved+retired, which in one sense fails to meet our memory-usage goals, but is necessary to retain the semantics (unresolved Promises do have an identity, and cannot be synthesized from plain data because we can't serialize any then callbacks that might be attached.. we can't even sense their existence). Rehydrating the virtual object, or doing a get on the WeakStore, will return the original Promise if it is still unresolved. If it is resolved, and thus retired, we need a place to track the resolved data, so the rehydrated version can be immediately resolved to the right state, rather than lingering forever as an unresolved promise.

Description of the Design

Not sure yet, probably forbid them entirely, but maybe there's a clever way to serialize the necessary data.

In general I'm thinking our serialization code should return the capdata and let the caller decide what .then calls need to be done. Perhaps it should return a list of unresolved promises in addition to the capdata (as opposed to strictly returning the slots and letting the caller dig through slotToVal to find the original objects).

Security Considerations

I suspect that any failures this could cause will be limited to the code that added the troublesome Promise. I don't expect it would spread beyond the original vat, or affect the kernel significantly. It might allow arbitrary code within a vat to kill the vat, if the spurious syscall.resolve is treated by the kernel as an illegal syscall.

cc @FUDCo

@warner warner added enhancement New feature or request SwingSet package: SwingSet labels Feb 19, 2021
@warner
Copy link
Member Author

warner commented Jul 23, 2021

I think @erights 's recent "virtual collections" work is heading towards a design where anything serializable can be used as data in the collection, and anything both serializable and having a stable identity can be used as a key or index. The latter requirement rules out the use of Promises there.

I floated the idea that maybe we could exclude Promises from being keys of the WeakMap/WeakSet we expose to userspace, because I'm worried that our retire-on-resolve behavior will allow them to be used as a GC sensor:

  • userspace receives a Promise, puts it in a WeakSet, otherwise drops the Promise
    • liveslots still retains it, until it is retired
  • userspace receives the same kpid, liveslots uses its Map to translate it into the same Promise, userspace recognizes it via the WeakSet
  • someone resolves the promise, liveslots retires the vpid and deletes the Map entry
  • userspace receives another copy of the same kpid, the kernel re-creates the vpid briefly, userspace gets a different Promise, can tell by looking at the WeakSet

Ok, when I write it up that way, userspace is sensing the resolution of the promise, not the collection of the Promise object, which is ok. I'll have to think about it further.

@erights said that forbidding Promises from being used as keys of (what look like) plain WeakMaps would be pretty drastic, so we'd prefer to not do that if at all possible.

@Tartuffo Tartuffo added MN-1 and removed MN-1 labels Feb 2, 2022
@warner
Copy link
Member Author

warner commented Feb 9, 2022

@FUDCo I know you and @erights have a whole scheme for what objects can be used in what contexts. You're closer to those docs than I am, so I'm assigning this to you two with the specific question of "can Promises be held in virtual or durable data?" (so virtual objects .state, durable objects .state, and virtual/durable collections).

Once you've got an answer, assign it back to me. If the answer is "no", I'll probably close this with some commentary. If the answer is "yes", then we've got a bigger problem to address.

I'll set the ticket size to "1" because I'm guessing (hoping?) the answer will be "no".

@FUDCo
Copy link
Contributor

FUDCo commented Feb 9, 2022

With respect to durable data, the answer is a definite "no", at least with respect to Promises per se, since promises don't survive the death and resurrection of a vat. Obviously if you hold a presence to a remote object that was imported, you can't actually tell if it's a promise on the remote end or not, but at that point you don't care because the remote vat's promise GC behavior is not your concern.

With respect to virtual data, it appears we do allow promises to be used as values in virtual objects and stores, but not as keys in stores. It's also not obvious to me upon closer inspection whether the latter exclusion is actually deliberate or simply an implementation artifact (i.e., just epsilon more respectable than a bug) that could be relaxed with a small tweak. @erights needs to weigh in on this.

@FUDCo FUDCo assigned warner and unassigned FUDCo Feb 9, 2022
@erights
Copy link
Member

erights commented Feb 9, 2022 via email

@Tartuffo
Copy link
Contributor

Per convo with Dean and Brian, we believe the answer is "NO". @warner please flesh this out.

@erights
Copy link
Member

erights commented Feb 22, 2022

can virtual objects or the WeakStore hold a Promise?

As stated, the answer is yes. But they can only be heap promises, and thus

  • of low cardinality
  • not durable-compatible, and so can only be stored in virtual objects and virtual stores, not durable objects and durable stores.

@Tartuffo
Copy link
Contributor

@dtribble for follow on discussion.

@warner
Copy link
Member Author

warner commented Feb 23, 2022

Current behavior:

  • putting a promise into virtualized data is not prohibited
    • doing so costs RAM
    • doing so might cause the problems I outlined in the top comment
  • putting a promise into durable data is prohibited
    • you get an exception during property assignment or map.set(key, promise), just after serialization, as we check the slots for durability

Next step is for me to think about the problems I outlined and make some test cases

  • store a promise in a virtual object, resolve it, see if that causes a spurious syscall.resolve
    • then send the promise into the kernel, see if that results in a never-resolved kernel promise
  • receive a promise from the kernel, store it in a virtual object, drop the representative, receive a resolution, create a new representative, add a .then to the promise, see if the .then fires
    • ideally the original Promise is kept around the whole time, but I think we delete it from slotToVal when resolved
    • we might need to keep it in the slotToVal WeakRef until it becomes unreferenced, even if it's resolved first, ugh

@warner
Copy link
Member Author

warner commented Feb 23, 2022

Leaving in MN-1 because this might enable GC nondeterminism from userspace.

@erights
Copy link
Member

erights commented Feb 23, 2022

I think I understand the concerns better now. I await the answers to your next steps investigation. Thanks.

@Tartuffo Tartuffo added this to the Mainnet 1 milestone Mar 23, 2022
@Tartuffo Tartuffo modified the milestones: Mainnet 1, RUN Protocol RC0 Apr 5, 2022
@warner
Copy link
Member Author

warner commented Jun 9, 2022

#5106 addressed all of these problems. Virtual (i.e. merely-virtual, i.e. non-durable) objects and collections can hold Promises, but they consume RAM. Durable objects/collections cannot hold Promises. Now that #5106 is fixed, we don't syscall.resolve into the kernel unless the promise was actually sent into the kernel, and we maintain the vpid/Promise registration (valToSlot/slotToVal) as long as (virtual data holds a reference) OR (the kernel knows about the promise) OR (a PromiseWatcher is paying attention to the promise). We drop the "kernel knows about the promise" part when it resolves, but that doesn't affect the vdata pillar.

@warner warner closed this as completed Jun 9, 2022
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

No branches or pull requests

4 participants