Skip to content

Commit

Permalink
fix: prevent marker facets from becoming orphaned
Browse files Browse the repository at this point in the history
  • Loading branch information
@FUDCo
FUDCo committed Mar 16, 2022
1 parent 00604bd commit cd6ca3a
Show file tree
Hide file tree
Showing 6 changed files with 185 additions and 79 deletions.
64 changes: 33 additions & 31 deletions packages/SwingSet/src/liveslots/virtualObjectManager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { Far } from '@endo/marshal';
* starting to throw them away.
* @param {(baseRef: string) => Object} fetch Function to retrieve an
* object's raw state from the store by its baseRef
* @param {(baseRef: string, rawData: Object) => void} store Function to
* @param {(baseRef: string, rawState: Object) => void} store Function to
* store raw object state by its baseRef
*
* @returns {Object} An LRU cache of (up to) the given size
Expand All @@ -32,11 +32,11 @@ export function makeCache(size, fetch, store) {
// kdebug(`### vo LRU evict ${lruTail.baseRef} (dirty=${lruTail.dirty})`);
liveTable.delete(lruTail.baseRef);
if (lruTail.dirty) {
store(lruTail.baseRef, lruTail.rawData);
store(lruTail.baseRef, lruTail.rawState);
lruTail.dirty = false;
dirtyCount -= 1;
}
lruTail.rawData = null;
lruTail.rawState = null;
if (lruTail.prev) {
lruTail.prev.next = undefined;
} else {
Expand All @@ -59,7 +59,7 @@ export function makeCache(size, fetch, store) {
let entry = lruTail;
while (entry) {
if (entry.dirty) {
store(entry.baseRef, entry.rawData);
store(entry.baseRef, entry.rawState);
entry.dirty = false;
}
entry = entry.prev;
Expand Down Expand Up @@ -110,11 +110,11 @@ export function makeCache(size, fetch, store) {
if (innerObj) {
cache.refresh(innerObj);
} else {
innerObj = { baseRef, rawData: null, repCount: 0 };
innerObj = { baseRef, rawState: null, repCount: 0 };
cache.remember(innerObj);
}
if (load && !innerObj.rawData) {
innerObj.rawData = fetch(baseRef);
if (load && !innerObj.rawState) {
innerObj.rawState = fetch(baseRef);
}
return innerObj;
},
Expand Down Expand Up @@ -185,6 +185,7 @@ export function makeVirtualObjectManager(
// a state object from being able to observe the comings and goings of
// representatives.
const stateToRepresentative = new WeakMap();
const facetToCohort = new WeakMap();

/**
* Fetch an object's state from secondary storage.
Expand All @@ -194,9 +195,9 @@ export function makeVirtualObjectManager(
* @returns {*} an object representing the object's stored state.
*/
function fetch(baseRef) {
const raw = syscall.vatstoreGet(`vom.${baseRef}`);
if (raw) {
return JSON.parse(raw);
const rawState = syscall.vatstoreGet(`vom.${baseRef}`);
if (rawState) {
return JSON.parse(rawState);
} else {
return undefined;
}
Expand All @@ -207,10 +208,10 @@ export function makeVirtualObjectManager(
*
* @param {string} baseRef The baseRef of the object whose state is being
* stored.
* @param {*} rawData A data object representing the state to be written.
* @param {*} rawState A data object representing the state to be written.
*/
function store(baseRef, rawData) {
syscall.vatstoreSet(`vom.${baseRef}`, JSON.stringify(rawData));
function store(baseRef, rawState) {
syscall.vatstoreSet(`vom.${baseRef}`, JSON.stringify(rawState));
}

/* eslint max-classes-per-file: ["error", 2] */
Expand Down Expand Up @@ -506,23 +507,23 @@ export function makeVirtualObjectManager(
}

function ensureState() {
if (innerSelf.rawData) {
if (innerSelf.rawState) {
cache.refresh(innerSelf);
} else {
innerSelf = cache.lookup(innerSelf.baseRef, true);
}
}

const wrappedData = {};
const wrappedState = {};
for (const prop of propertyNames) {
Object.defineProperty(wrappedData, prop, {
Object.defineProperty(wrappedState, prop, {
get: () => {
ensureState();
return unserialize(innerSelf.rawData[prop]);
return unserialize(innerSelf.rawState[prop]);
},
set: value => {
ensureState();
const before = innerSelf.rawData[prop];
const before = innerSelf.rawState[prop];
const after = serialize(value);
if (durable) {
after.slots.map(vref =>
Expand All @@ -533,17 +534,17 @@ export function makeVirtualObjectManager(
);
}
vrm.updateReferenceCounts(before.slots, after.slots);
innerSelf.rawData[prop] = after;
innerSelf.rawState[prop] = after;
cache.markDirty(innerSelf);
},
});
}
harden(wrappedData);
harden(wrappedState);

if (initializing) {
cache.remember(innerSelf);
}
const self = actualize(wrappedData);
const self = actualize(wrappedState);
let toHold;
let toExpose;
const facetiousness = assessFacetiousness(self);
Expand All @@ -567,6 +568,7 @@ export function makeVirtualObjectManager(
const facet = Far(`${tag} ${facetName}`, self[facetName]);
toExpose[facetName] = facet;
toHold.push(facet);
facetToCohort.set(facet, toHold);
}
harden(toExpose);
break;
Expand All @@ -578,9 +580,9 @@ export function makeVirtualObjectManager(
}
if (!proForma) {
innerSelf.representative = toHold;
stateToRepresentative.set(wrappedData, toHold);
stateToRepresentative.set(wrappedState, toHold);
}
return [toHold, toExpose, wrappedData];
return [toHold, toExpose, wrappedState];
}

function reanimate(baseRef, proForma) {
Expand All @@ -596,9 +598,9 @@ export function makeVirtualObjectManager(

function deleteStoredVO(baseRef) {
let doMoreGC = false;
const state = fetch(baseRef);
if (state) {
for (const propValue of Object.values(state)) {
const rawState = fetch(baseRef);
if (rawState) {
for (const propValue of Object.values(rawState)) {
propValue.slots.map(
vref => (doMoreGC = doMoreGC || vrm.removeReachableVref(vref)),
);
Expand All @@ -616,7 +618,7 @@ export function makeVirtualObjectManager(
// kdebug(`vo make ${baseRef}`);

const initialData = init ? init(...args) : {};
const rawData = {};
const rawState = {};
for (const prop of Object.getOwnPropertyNames(initialData)) {
const data = serialize(initialData[prop]);
if (durable) {
Expand All @@ -625,18 +627,18 @@ export function makeVirtualObjectManager(
);
}
data.slots.map(vrm.addReachableVref);
rawData[prop] = data;
rawState[prop] = data;
propertyNames.add(prop);
}
const innerSelf = { baseRef, rawData, repCount: 0 };
const [toHold, toExpose, wrappedData] = makeRepresentative(
const innerSelf = { baseRef, rawState, repCount: 0 };
const [toHold, toExpose, state] = makeRepresentative(
innerSelf,
true,
false,
);
registerValue(baseRef, toHold, Array.isArray(toHold));
if (finish) {
finish(wrappedData, toExpose);
finish(state, toExpose);
}
cache.markDirty(innerSelf);
return toExpose;
Expand Down
30 changes: 30 additions & 0 deletions packages/SwingSet/test/virtualObjects/test-representatives.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,3 +391,33 @@ test('virtual object gc', async t => {
'v1.vs.vom.o+1/9': '{"label":{"body":"\\"thing #9\\"","slots":[]}}',
});
});

test('empty facets are not orphaned', async t => {
const config = {
bootstrap: 'bootstrap',
vats: {
bob: {
sourceSpec: new URL('vat-orphan-bob.js', import.meta.url).pathname,
creationOptions: {
virtualObjectCacheSize: 0,
},
},
bootstrap: {
sourceSpec: new URL('vat-orphan-bootstrap.js', import.meta.url)
.pathname,
},
},
};

const hostStorage = provideHostStorage();

const c = await buildVatController(config, [], { hostStorage });
c.pinVatRoot('bootstrap');

await c.run();
t.deepEqual(
c.kpResolution(c.bootstrapResult),
capargs({ '@qclass': 'undefined' }),
);
t.deepEqual(c.dump().log, ['compare originalFacet === thing : true']);
});
54 changes: 27 additions & 27 deletions packages/SwingSet/test/virtualObjects/test-virtualObjectCache.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function makeThing(n) {
// object above it
return {
baseRef: `t${n}`,
rawData: `thing #${n}`,
rawState: `thing #${n}`,
dirty: false,
};
}
Expand All @@ -55,35 +55,35 @@ test('cache overflow and refresh', t => {
// cache: t5, t4, t3, t2

// after initialization
t.is(things[0].rawData, null);
t.is(things[1].rawData, null);
t.is(things[2].rawData, 'thing #2');
t.is(things[3].rawData, 'thing #3');
t.is(things[4].rawData, 'thing #4');
t.is(things[5].rawData, 'thing #5');
t.is(things[0].rawState, null);
t.is(things[1].rawState, null);
t.is(things[2].rawState, 'thing #2');
t.is(things[3].rawState, 'thing #3');
t.is(things[4].rawState, 'thing #4');
t.is(things[5].rawState, 'thing #5');
t.deepEqual(store.getLog(), [
['store', 't0', 'thing #0'],
['store', 't1', 'thing #1'],
]);

// lookup that refreshes
cache.lookup('t2'); // cache: t2, t5, t4, t3
t.is(things[5].rawData, 'thing #5');
t.is(things[5].rawState, 'thing #5');
t.deepEqual(store.getLog(), []);

// lookup that has no effect
things[0] = cache.lookup('t0'); // cache: t0, t2, t5, t4
things[0].rawData = 'changed thing #0';
things[0].rawState = 'changed thing #0';
cache.markDirty(things[0]); // pretend we changed it
t.is(things[0].rawData, 'changed thing #0');
t.is(things[3].rawData, null);
t.is(things[0].rawState, 'changed thing #0');
t.is(things[3].rawState, null);
t.deepEqual(store.getLog(), [['store', 't3', 'thing #3']]);

// verify refresh
cache.refresh(things[4]); // cache: t4, t0, t2, t5
things[1] = cache.lookup('t1'); // cache: t1, t4, t0, t2
t.is(things[1].rawData, null);
t.is(things[5].rawData, null);
t.is(things[1].rawState, null);
t.is(things[5].rawState, null);
t.deepEqual(store.getLog(), [['store', 't5', 'thing #5']]);

// verify that everything is there
Expand All @@ -100,12 +100,12 @@ test('cache overflow and refresh', t => {
t.falsy(things[3].dirty);
t.falsy(things[4].dirty);
t.falsy(things[5].dirty);
t.is(things[0].rawData, 'changed thing #0');
t.is(things[1].rawData, null);
t.is(things[2].rawData, 'thing #2');
t.is(things[3].rawData, null);
t.is(things[4].rawData, 'thing #4');
t.is(things[5].rawData, null);
t.is(things[0].rawState, 'changed thing #0');
t.is(things[1].rawState, null);
t.is(things[2].rawState, 'thing #2');
t.is(things[3].rawState, null);
t.is(things[4].rawState, 'thing #4');
t.is(things[5].rawState, null);
t.deepEqual(store.getLog(), [
['store', 't2', 'thing #2'],
['store', 't0', 'changed thing #0'],
Expand All @@ -122,25 +122,25 @@ test('cache overflow and refresh', t => {

// verify that changes get written
things[0] = cache.lookup('t0'); // cache: t0
things[0].rawData = 'new thing #0';
things[0].rawState = 'new thing #0';
things[0].dirty = true;
things[1] = cache.lookup('t1'); // cache: t1, t0
things[1].rawData = 'new thing #1';
things[1].rawState = 'new thing #1';
things[1].dirty = true;
things[2] = cache.lookup('t2'); // cache: t2, t1, t0
things[2].rawData = 'new thing #2';
things[2].rawState = 'new thing #2';
things[2].dirty = true;
things[3] = cache.lookup('t3'); // cache: t3, t2, t1, t0
things[3].rawData = 'new thing #3';
things[3].rawState = 'new thing #3';
things[3].dirty = true;
things[4] = cache.lookup('t4'); // cache: t4, t3, t2, t1
things[4].rawData = 'new thing #4';
things[4].rawState = 'new thing #4';
things[4].dirty = true;
things[5] = cache.lookup('t5'); // cache: t5, t4, t3, t2
things[5].rawData = 'new thing #5';
things[5].rawState = 'new thing #5';
things[5].dirty = true;
t.is(things[0].rawData, null);
t.is(things[5].rawData, 'new thing #5');
t.is(things[0].rawState, null);
t.is(things[5].rawState, 'new thing #5');
t.deepEqual(store.getLog(), [
['store', 't0', 'new thing #0'],
['store', 't1', 'new thing #1'],
Expand Down
Loading

0 comments on commit cd6ca3a

Please sign in to comment.