Skip to content

Commit

Permalink
fix(swingset): track exported Remotables during export, not serializa…
Browse files Browse the repository at this point in the history
…tion

The `convertValToSlot` function is called during `m.serialize`, which happens
in three places:

* message send, just before `syscall.send()`
* promise resolution into kernel, just before `syscall.resolve()`
* when serializing virtualized data:
  * writing to the property of a virtual object
  * saving data to a value of a vref-keyed `makeWeakStore()` instance

Remotables are assigned a vref during serialization. However we need to track
two different kinds of reachability:

* reachable by kernel:
  * this flag is set when syscall.send or syscall.resolve includes the
    Remotable's vref in the arguments
  * the flag is cleared when the kernel calls `dispatch.dropExport`
* reachable by virtualized data:
  * this is flag is set when a virtual-object property setter or
    weakstore.init/set sees the vref in the serialized virtualized data
  * the flag is cleared when the virtualized data's last reference goes away

We must retain a strong reference to the Remotable as long as either form of
reachability is possible. The virtual object manager uses the
`reachableRemotables` Set to retain this reference for everything in
virtualized data. Liveslots uses the `exportedRemotables` Set to do the same
for things that have been exported into the kernel.

However, previously liveslots added things to `exportedRemotables` during
*serialization*, which was wrong because the same serialization function is
also used by virtualized data. So this patch changes liveslots to add the
Remotable just *after* serialization in the two specific places that send
things into the kernel: `syscall.send` and `syscall.resolve`.

The failure mode this should fix would be:

* vat creates a Remotable
* vat stores Remotable into virtualized data
  * the old `convertValToSlot` would add Remotable to `exportedRemotables`
  * but nothing actually sent it into the kernel
  * VOM would also (correctly) add Remotable to `reachableRemotables`
* vat drops strong reference to Remotable
  * leaving two strongrefs: `reachableRemotables` (correct),
    `exportedRemotables` (wrong)
* virtualized data is deleted (e.g. `weakstore.delete()`, or state overwrite)
  * eventually (when we implement refcounting in the VOM), this will
    delete the strongref from `reachableRemotables`
* then failure is that the Remotable is not released, because
  `exportedRemotables` still references it, and the kernel will never do
  `dispatch.dropExport` because the kernel never saw it

I won't be able to test this until we implement more precise reachability
tracking for virtualized data, which is going to be a while.
  • Loading branch information
warner committed May 25, 2021
1 parent 2a07d8e commit 0bc31e9
Showing 1 changed file with 29 additions and 9 deletions.
38 changes: 29 additions & 9 deletions packages/SwingSet/src/kernel/liveSlots.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,17 @@ function build(
const safetyPins = new Set(); // temporary
const deadSet = new Set(); // vrefs that are finalized but not yet reported

function retainExportedRemotable(vref) {
// if the vref corresponds to a Remotable, keep a strong reference to it
// until the kernel tells us to release it
const { type, allocatedByVat, virtual } = parseVatSlot(vref);
if (type === 'object' && allocatedByVat && !virtual) {
const remotable = slotToVal.get(vref).deref();
assert(remotable, X`somehow lost Remotable for ${vref}`);
exportedRemotables.add(remotable);
}
}

/*
Imports are in one of 5 states: UNKNOWN, REACHABLE, UNREACHABLE,
COLLECTED, FINALIZED. Note that there's no actual state machine with those
Expand Down Expand Up @@ -395,11 +406,13 @@ function build(
assert.equal(passStyleOf(val), 'remotable');
slot = exportPassByPresence();
}
parseVatSlot(slot); // assertion
const { type } = parseVatSlot(slot); // also used as assertion
valToSlot.set(val, slot);
slotToVal.set(slot, new WeakRef(val));
// we do not use droppedRegistry for exports
exportedRemotables.add(val); // keep it alive until kernel tells us to release it
if (type === 'object') {
deadSet.delete(slot);
droppedRegistry.register(val, slot);
}
}
return valToSlot.get(val);
}
Expand All @@ -418,10 +431,10 @@ function build(
const { type, virtual } = parseVatSlot(slot);
slotToVal.set(slot, new WeakRef(val));
valToSlot.set(val, slot);
if (type === 'object' || type === 'device') {
// we don't dropImports on promises, to avoid interaction with retire
droppedRegistry.register(val, slot);
// we don't dropImports on promises, to avoid interaction with retire
if (type === 'object') {
deadSet.delete(slot); // might have been FINALIZED before, no longer
droppedRegistry.register(val, slot);
}

// TODO: until #2724 is implemented, we cannot actually release
Expand Down Expand Up @@ -521,6 +534,7 @@ function build(
function collect(promiseID, rejected, value) {
doneResolutions.add(promiseID);
const valueSer = m.serialize(value);
valueSer.slots.map(retainExportedRemotable);
resolutions.push([promiseID, rejected, valueSer]);
scanSlots(valueSer.slots);
}
Expand Down Expand Up @@ -552,6 +566,7 @@ function build(
}

const serArgs = m.serialize(harden(args));
serArgs.slots.map(retainExportedRemotable);
const resultVPID = allocatePromiseID();
lsdebug(`Promise allocation ${forVatID}:${resultVPID} in queueMessage`);
// create a Promise which callers follow for the result, give it a
Expand Down Expand Up @@ -609,6 +624,7 @@ function build(
}
return (...args) => {
const serArgs = m.serialize(harden(args));
serArgs.slots.map(retainExportedRemotable);
forbidPromises(serArgs);
const ret = syscall.callNow(slot, prop, serArgs);
insistCapData(ret);
Expand Down Expand Up @@ -802,11 +818,15 @@ function build(
// TODO: when we add notifyForward, guard against cycles

function exitVat(completion) {
syscall.exit(false, m.serialize(harden(completion)));
const args = m.serialize(harden(completion));
args.slots.map(retainExportedRemotable);
syscall.exit(false, args);
}

function exitVatWithFailure(reason) {
syscall.exit(true, m.serialize(harden(reason)));
const args = m.serialize(harden(reason));
args.slots.map(retainExportedRemotable);
syscall.exit(true, args);
}

function disavow(presence) {
Expand Down Expand Up @@ -858,8 +878,8 @@ function build(
const rootSlot = makeVatSlot('object', true, BigInt(0));
valToSlot.set(rootObject, rootSlot);
slotToVal.set(rootSlot, new WeakRef(rootObject));
retainExportedRemotable(rootSlot);
// we do not use droppedRegistry for exports
exportedRemotables.add(rootObject);
}

/**
Expand Down

0 comments on commit 0bc31e9

Please sign in to comment.