From 133bbae35ac08f3883380682944ffd606ba638bf Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Thu, 17 Jun 2021 11:44:30 -0700 Subject: [PATCH] feat(swingset): comms: add/manipulate isReachable flag Each c-list object entry now has an `isReachable` flag. On import-facing c-lists, this indicates that the downstream (kernel or remote) can reach the object (and is cleared when the downstream side drops their strong reference). On the one export-facing c-list, we use the flag to remember whether we've dropped our import or not. c-list translators now ensure the flag is set after an allocation-worthy reference (imports during inbound translation, exports during outbound translation) is converted. C-list entry deletion functions ensure the flag is clear. In either case, if the state of the flag actually changed (on an import-side c-list), the `reachable` refcount is modified. If that refcount touches zero, the object is marked as "maybe free" for later investigation. `addEgress` also sets the isReachable flag, to reflect the initial conditions of a manually-added c-list entry. --- .../SwingSet/src/vats/comms/clist-inbound.js | 32 ++++++++-- .../SwingSet/src/vats/comms/clist-kernel.js | 52 +++++++++++++++-- .../SwingSet/src/vats/comms/clist-outbound.js | 34 +++++++++-- .../SwingSet/src/vats/comms/clist-xgress.js | 3 + packages/SwingSet/src/vats/comms/remote.js | 56 +++++++++++++++++- packages/SwingSet/src/vats/comms/state.js | 58 ++++++++++++++++++- 6 files changed, 214 insertions(+), 21 deletions(-) diff --git a/packages/SwingSet/src/vats/comms/clist-inbound.js b/packages/SwingSet/src/vats/comms/clist-inbound.js index d72e400a581..a1739d619c0 100644 --- a/packages/SwingSet/src/vats/comms/clist-inbound.js +++ b/packages/SwingSet/src/vats/comms/clist-inbound.js @@ -49,8 +49,12 @@ export function makeInbound(state) { function getLocalForRemote(remoteID, rref) { const remote = state.getRemote(remoteID); - const lref = remote.mapFromRemote(rref); + const { mapFromRemote, isReachable } = remote; + const lref = mapFromRemote(rref); assert(lref, X`${rref} must already be in remote ${rname(remote)}`); + if (parseRemoteSlot(rref).type === 'object') { + assert(isReachable(lref), `remote sending to unreachable ${lref}`); + } return lref; } @@ -93,8 +97,10 @@ export function makeInbound(state) { // previously, or if we're the ones who sent it to them earlier, it will be // in the inbound table already. const remote = state.getRemote(remoteID); - if (!remote.mapFromRemote(rref)) { - const { type } = parseRemoteSlot(rref); + const { type, allocatedByRecipient } = parseRemoteSlot(rref); + // !allocatedByRecipient means we're willing to allocate + let lref = remote.mapFromRemote(rref); + if (!lref) { if (type === 'object') { addLocalObjectForRemote(remote, rref); } else if (type === 'promise') { @@ -102,8 +108,26 @@ export function makeInbound(state) { } else { assert.fail(X`cannot accept type ${type} from remote`); } + lref = remote.mapFromRemote(rref); + } + + // in either case, we need to mark imports or re-imports as reachable + if (type === 'object') { + // Senders are very polite and always translate rrefs into the + // recipient's number space. So if we receive ro-2 from the remote + // (!allocatedByRecipient), that means it was allocated by the remote, + // and this is an exporting reference. We're willing to allocate an + // lref on this inbound pathway. + const doSetReachable = !allocatedByRecipient; + if (doSetReachable) { + // the remote is exporting, not importing + const isImport = false; + remote.setReachable(lref, isImport); + } + assert(remote.isReachable(lref), `remote using unreachable ${lref}`); } - return remote.mapFromRemote(rref); + + return lref; } function provideLocalForRemoteResult(remoteID, result) { diff --git a/packages/SwingSet/src/vats/comms/clist-kernel.js b/packages/SwingSet/src/vats/comms/clist-kernel.js index 016702b63a5..039b925a740 100644 --- a/packages/SwingSet/src/vats/comms/clist-kernel.js +++ b/packages/SwingSet/src/vats/comms/clist-kernel.js @@ -15,17 +15,25 @@ export function makeKernel(state, syscall) { // been retired or not, and create a new (short-lived) identifier to reference // resolved promises if necessary. + const isReachable = state.isReachableByKernel; + const setReachable = state.setReachableByKernel; + // const clearReachable = state.clearReachableByKernel; + function getKernelForLocal(lref) { const kfref = state.mapToKernel(lref); assert(kfref, X`${lref} must already be mapped to a kernel-facing ID`); + const { type, allocatedByVat } = parseVatSlot(kfref); + if (type === 'object' && !allocatedByVat) { + // comms import, kernel export, make sure we can stil reach it + assert(isReachable(lref), X`comms sending to unreachable import ${lref}`); + } return kfref; } function provideKernelForLocal(lref) { - if (!state.mapToKernel(lref)) { - let kfref; - const { type } = parseLocalSlot(lref); - + const { type } = parseLocalSlot(lref); + let kfref = state.mapToKernel(lref); + if (!kfref) { if (type === 'object') { kfref = state.allocateKernelObjectID(); } else if (type === 'promise') { @@ -44,7 +52,21 @@ export function makeKernel(state, syscall) { state.addKernelMapping(kfref, lref); cdebug(`comms add mapping l->k ${kfref}<=>${lref}`); } - return state.mapToKernel(lref); + + if (type === 'object') { + // we setReachable in the same cases that we're willing to allocate a + // kfref + const { allocatedByVat } = parseVatSlot(kfref); + const doSetReachable = allocatedByVat; + if (doSetReachable) { + // the kernel is an importer in this case + const isImport = true; + setReachable(lref, isImport); + } + assert(isReachable(lref), X`comms sending unreachable ${lref}`); + } + + return kfref; } function provideKernelForLocalResult(lpid) { @@ -81,6 +103,13 @@ export function makeKernel(state, syscall) { function getLocalForKernel(kfref) { const lref = state.mapFromKernel(kfref); assert(lref, X`${kfref} must already be mapped to a local ID`); + if (parseVatSlot(kfref).type === 'object') { + // comms export, kernel import, it must be reachable + assert( + isReachable(lref), + X`kernel sending to unreachable import ${lref}`, + ); + } return lref; } @@ -123,15 +152,26 @@ export function makeKernel(state, syscall) { assert.fail(X`cannot accept type ${type} from kernel`); } } - const lref = state.mapFromKernel(kfref); + if (type === 'promise') { if (!allocatedByVat) { if (!doNotSubscribeSet || !doNotSubscribeSet.has(kfref)) { syscall.subscribe(kfref); } } + } else if (type === 'object') { + // we setReachable in the same cases that we're willing to allocate an + // lref + const doSetReachable = !allocatedByVat; + if (doSetReachable) { + // the kernel is an exporter in this case + const isImport = false; + setReachable(lref, isImport); + } + assert(isReachable(lref), `kernel using unreachable ${lref}`); } + return lref; } diff --git a/packages/SwingSet/src/vats/comms/clist-outbound.js b/packages/SwingSet/src/vats/comms/clist-outbound.js index 08ac7a3548e..2063abfba2f 100644 --- a/packages/SwingSet/src/vats/comms/clist-outbound.js +++ b/packages/SwingSet/src/vats/comms/clist-outbound.js @@ -1,6 +1,10 @@ import { assert, details as X } from '@agoric/assert'; import { parseLocalSlot, insistLocalType } from './parseLocalSlots.js'; -import { flipRemoteSlot, insistRemoteType } from './parseRemoteSlot.js'; +import { + flipRemoteSlot, + insistRemoteType, + parseRemoteSlot, +} from './parseRemoteSlot.js'; import { cdebug } from './cdebug.js'; function rname(remote) { @@ -10,8 +14,12 @@ function rname(remote) { export function makeOutbound(state) { function getRemoteForLocal(remoteID, lref) { const remote = state.getRemote(remoteID); - const rref = remote.mapToRemote(lref); + const { mapToRemote, isReachable } = remote; + const rref = mapToRemote(lref); assert(rref, X`${lref} must already be in remote ${rname(remote)}`); + if (parseRemoteSlot(rref).type === 'object') { + assert(isReachable(lref), `sending unreachable ${lref} to remote`); + } return rref; } @@ -52,8 +60,9 @@ export function makeOutbound(state) { // or if they're the ones who sent it to us in the first place, it will be // in the outbound table already. const remote = state.getRemote(remoteID); - if (!remote.mapToRemote(lref)) { - const { type } = parseLocalSlot(lref); + const { type } = parseLocalSlot(lref); + let rref = remote.mapToRemote(lref); + if (!rref) { if (type === 'object') { addRemoteObjectForLocal(remote, lref); } else if (type === 'promise') { @@ -61,8 +70,23 @@ export function makeOutbound(state) { } else { assert.fail(X`cannot send type ${type} to remote`); } + rref = remote.mapToRemote(lref); } - const rref = remote.mapToRemote(lref); + + // in either case, we need to mark exports or re-exports as reachable + if (type === 'object') { + const { allocatedByRecipient } = parseRemoteSlot(rref); + // `rref` is what the remote wants to hear, so allocatedByRecipient + // means they exported it, !allocatedByRecipient means we did + const doSetReachable = !allocatedByRecipient; + if (doSetReachable) { + // the remote is always importing it + const isImport = true; + remote.setReachable(lref, isImport); + } + assert(remote.isReachable(lref), `sending unreachable rref ${lref}`); + } + return rref; } diff --git a/packages/SwingSet/src/vats/comms/clist-xgress.js b/packages/SwingSet/src/vats/comms/clist-xgress.js index 5e4ea4d6a49..5366bab77ac 100644 --- a/packages/SwingSet/src/vats/comms/clist-xgress.js +++ b/packages/SwingSet/src/vats/comms/clist-xgress.js @@ -16,6 +16,9 @@ export function makeIngressEgress(state, provideLocalForRemote) { const inboundRRef = makeRemoteSlot('object', true, remoteRefID); remote.addRemoteMapping(inboundRRef, loid); remote.skipRemoteObjectID(remoteRefID); + const isCommsImport = false; + remote.setReachable(loid, isCommsImport); + // prettier-ignore cdebug(`comms add egress ${loid} to ${remoteID} in ${inboundRRef}`); } diff --git a/packages/SwingSet/src/vats/comms/remote.js b/packages/SwingSet/src/vats/comms/remote.js index 8ed798cbe4e..3664b53e7a3 100644 --- a/packages/SwingSet/src/vats/comms/remote.js +++ b/packages/SwingSet/src/vats/comms/remote.js @@ -1,6 +1,11 @@ import { Nat } from '@agoric/nat'; import { assert, details as X } from '@agoric/assert'; -import { makeRemoteSlot, flipRemoteSlot } from './parseRemoteSlot.js'; +import { parseLocalSlot } from './parseLocalSlots.js'; +import { + makeRemoteSlot, + flipRemoteSlot, + parseRemoteSlot, +} from './parseRemoteSlot.js'; export function insistRemoteID(remoteID) { assert(/^r\d+$/.test(remoteID), X`not a remoteID: ${remoteID}`); @@ -51,22 +56,62 @@ export function makeRemote(state, store, remoteID) { return store.get(`${remoteID}.c.${lref}`); } + // is/set/clear are used on both imports and exports, but set/clear needs + // to be told which one it is + + function isReachable(lref) { + assert.equal(parseLocalSlot(lref).type, 'object'); + return !!store.get(`${remoteID}.cr.${lref}`); + } + function setReachable(lref, isImport) { + const wasReachable = isReachable(lref); + if (!wasReachable) { + store.set(`${remoteID}.cr.${lref}`, `1`); + if (isImport) { + state.changeReachable(lref, 1n); + } + } + } + function clearReachable(lref, isImport) { + const wasReachable = isReachable(lref); + if (wasReachable) { + store.delete(`${remoteID}.cr.${lref}`); + if (isImport) { + const reachable = state.changeReachable(lref, -1n); + if (!reachable) { + state.lrefMightBeFree(lref); + } + } + } + } + + // rref is what we would get from them, so + means our export function addRemoteMapping(rref, lref) { + const { type, allocatedByRecipient } = parseRemoteSlot(rref); + const isImport = allocatedByRecipient; const fromKey = `${remoteID}.c.${rref}`; const toKey = `${remoteID}.c.${lref}`; assert(!store.get(fromKey), X`already have ${rref}`); assert(!store.get(toKey), X`already have ${lref}`); store.set(fromKey, lref); store.set(toKey, flipRemoteSlot(rref)); - state.incrementRefCount(lref, `{rref}|${remoteID}|clist`); + const mode = isImport ? 'clist-import' : 'clist-export'; + state.incrementRefCount(lref, `{rref}|${remoteID}|clist`, mode); } function deleteRemoteMapping(lref) { const rrefOutbound = store.get(`${remoteID}.c.${lref}`); const rrefInbound = flipRemoteSlot(rrefOutbound); + let mode = 'data'; // close enough + const { type, allocatedByRecipient } = parseRemoteSlot(rrefInbound); + const isImport = allocatedByRecipient; + if (type === 'object') { + clearReachable(lref, isImport); + mode = isImport ? 'clist-import' : 'clist-export'; + } store.delete(`${remoteID}.c.${rrefInbound}`); store.delete(`${remoteID}.c.${lref}`); - state.decrementRefCount(lref, `{rref}|${remoteID}|clist`); + state.decrementRefCount(lref, `{rref}|${remoteID}|clist`, mode); } function nextSendSeqNum() { @@ -144,10 +189,15 @@ export function makeRemote(state, store, remoteID) { return harden({ remoteID: () => remoteID, name, + mapFromRemote, mapToRemote, + isReachable, + setReachable, + clearReachable, addRemoteMapping, deleteRemoteMapping, + allocateRemoteObject, skipRemoteObjectID, allocateRemotePromise, diff --git a/packages/SwingSet/src/vats/comms/state.js b/packages/SwingSet/src/vats/comms/state.js index 90416ac24c6..e360f5fab7c 100644 --- a/packages/SwingSet/src/vats/comms/state.js +++ b/packages/SwingSet/src/vats/comms/state.js @@ -1,7 +1,11 @@ import { Nat } from '@agoric/nat'; import { assert, details as X } from '@agoric/assert'; import { insistCapData } from '../../capdata.js'; -import { makeVatSlot, insistVatType } from '../../parseVatSlots.js'; +import { + makeVatSlot, + insistVatType, + parseVatSlot, +} from '../../parseVatSlots.js'; import { makeLocalSlot, parseLocalSlot } from './parseLocalSlots.js'; import { initializeRemoteState, makeRemote, insistRemoteID } from './remote.js'; import { cdebug } from './cdebug.js'; @@ -83,6 +87,7 @@ export function makeState(syscall, identifierBase = 0) { // p.nextID = $NN // kernel-facing promise identifier allocation counter (p+NN) // c.$kfref = $lref // inbound kernel-facing c-list (o+NN/o-NN/p+NN/p-NN -> loNN/lpNN) // c.$lref = $kfref // outbound kernel-facing c-list (loNN/lpNN -> o+NN/o-NN/p+NN/p-NN) + // cr.$lref = 1 | // isReachable flag // meta.$kfref = true // flag that $kfref (o+NN/o-NN) is a directly addressable control object // // lo.nextID = $NN // local object identifier allocation counter (loNN) @@ -110,6 +115,7 @@ export function makeState(syscall, identifierBase = 0) { // r$NN.transmitterID = $kfref // transmitter object for sending to remote // r$NN.c.$rref = $lref // r$NN inbound c-list (ro+NN/ro-NN/rp+NN/rp-NN -> loNN/lpNN) // r$NN.c.$lref = $rref // r$NN outbound c-list (loNN/lpNN -> ro+NN/ro-NN/rp+NN/rp-NN) + // r$NN.cr.$lref = 1 | // isReachable flag // r$NN.sendSeq = $NN // counter for outbound message sequence numbers to r$NN // r$NN.recvSeq = $NN // counter for inbound message sequence numbers from r$NN // r$NN.o.nextID = $NN // r$NN object identifier allocation counter (ro-NN) @@ -310,17 +316,60 @@ export function makeState(syscall, identifierBase = 0) { return store.get(`c.${lref}`); } + // is/set/clear are used on both imports and exports, but set/clear needs + // to be told which one it is + + function isReachableByKernel(lref) { + assert.equal(parseLocalSlot(lref).type, 'object'); + return !!store.get(`cr.${lref}`); + } + function setReachableByKernel(lref, isImport) { + const wasReachable = isReachableByKernel(lref); + if (!wasReachable) { + store.set(`cr.${lref}`, `1`); + if (isImport) { + changeReachable(lref, 1n); + } + } + } + function clearReachableByKernel(lref, isImport) { + const wasReachable = isReachableByKernel(lref); + if (wasReachable) { + store.delete(`cr.${lref}`); + if (isImport) { + const reachable = changeReachable(lref, -1n); + if (!reachable) { + maybeFree.add(lref); + } + } + } + } + + // translators should addKernelMapping for new imports/exports, then + // setReachableByKernel function addKernelMapping(kfref, lref) { + const { type, allocatedByVat } = parseVatSlot(kfref); + const isImport = allocatedByVat; // true = kernel is downstream importer store.set(`c.${kfref}`, lref); store.set(`c.${lref}`, kfref); - incrementRefCount(lref, `{kfref}|k|clist`); + const mode = isImport ? 'clist-import' : 'clist-export'; + incrementRefCount(lref, `{kfref}|k|clist`, mode); } + // GC or delete-remote should just call deleteKernelMapping without any + // extra clearReachableByKernel function deleteKernelMapping(lref) { const kfref = store.get(`c.${lref}`); + let mode = 'data'; // close enough + const { type, allocatedByVat } = parseVatSlot(kfref); + const isImport = allocatedByVat; + if (type === 'object') { + clearReachableByKernel(lref, isImport); + mode = isImport ? 'clist-import' : 'clist-export'; + } store.delete(`c.${kfref}`); store.delete(`c.${lref}`); - decrementRefCount(lref, `{kfref}|k|clist`); + decrementRefCount(lref, `{kfref}|k|clist`, mode); } function hasMetaObject(kfref) { @@ -552,6 +601,9 @@ export function makeState(syscall, identifierBase = 0) { mapFromKernel, mapToKernel, + isReachableByKernel, + setReachableByKernel, + clearReachableByKernel, addKernelMapping, deleteKernelMapping,