-
Notifications
You must be signed in to change notification settings - Fork 205
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
Properly handle remotables vs presences in weak collections #3142
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,163 @@ | ||
/* global __dirname globalThis */ | ||
import { test } from '../../tools/prepare-test-env-ava'; | ||
|
||
// eslint-disable-next-line import/order | ||
import path from 'path'; | ||
|
||
import { provideHostStorage } from '../../src/hostStorage'; | ||
import { initializeSwingset, makeSwingsetController } from '../../src/index'; | ||
import { makeFakeVirtualObjectManager } from '../../tools/fakeVirtualObjectManager'; | ||
import makeNextLog from '../make-nextlog'; | ||
|
||
function capdata(body, slots = []) { | ||
return harden({ body, slots }); | ||
} | ||
|
||
function capargs(args, slots = []) { | ||
return capdata(JSON.stringify(args), slots); | ||
} | ||
|
||
let gc; | ||
function insistGC(t) { | ||
if (globalThis.gc) { | ||
gc = globalThis.gc; | ||
} else { | ||
t.fail(`GC needs to be enabled for this test to work`); | ||
} | ||
} | ||
|
||
test('weakMap in vat', async t => { | ||
insistGC(t); | ||
const config = { | ||
bootstrap: 'bootstrap', | ||
defaultManagerType: 'local', | ||
vats: { | ||
bootstrap: { | ||
sourceSpec: path.resolve(__dirname, 'vat-weakcollections-bootstrap.js'), | ||
}, | ||
alice: { | ||
sourceSpec: path.resolve(__dirname, 'vat-weakcollections-alice.js'), | ||
}, | ||
}, | ||
}; | ||
|
||
const hostStorage = provideHostStorage(); | ||
const bootstrapResult = await initializeSwingset(config, [], hostStorage); | ||
const c = await makeSwingsetController(hostStorage, {}); | ||
const nextLog = makeNextLog(c); | ||
|
||
await c.run(); | ||
t.deepEqual(c.kpResolution(bootstrapResult), capargs('bootstrap done')); | ||
|
||
async function doSimple(method) { | ||
const sendArgs = capargs([], []); | ||
const r = c.queueToVatExport('bootstrap', 'o+0', method, sendArgs); | ||
await c.run(); | ||
t.is(c.kpStatus(r), 'fulfilled'); | ||
return c.kpResolution(r); | ||
} | ||
|
||
const preGCResult = await doSimple('runProbes'); | ||
t.deepEqual(preGCResult, capargs('probes done')); | ||
t.deepEqual(nextLog(), [ | ||
'probe of sample-object returns imported item #0', | ||
'probe of [object Promise] returns imported item #1', | ||
'probe of [Alleged: remember-exp] returns mer', | ||
'probe of [Alleged: holder-vo] returns mevo', | ||
'probe of [object Promise] returns mep', | ||
'probe of [Alleged: forget-exp] returns fer', | ||
'probe of [Alleged: holder-vo] returns fevo', | ||
'probe of [object Promise] returns fep', | ||
]); | ||
await doSimple('betweenProbes'); | ||
gc(); | ||
const postGCResult = await doSimple('runProbes'); | ||
t.deepEqual(postGCResult, capargs('probes done')); | ||
t.deepEqual(nextLog(), [ | ||
'probe of sample-object returns imported item #0', | ||
'probe of [object Promise] returns undefined', | ||
'probe of [Alleged: remember-exp] returns mer', | ||
'probe of [Alleged: holder-vo] returns mevo', | ||
'probe of [object Promise] returns mep', | ||
'probe of [Alleged: forget-exp] returns fer', | ||
'probe of [Alleged: holder-vo] returns fevo', | ||
'probe of [object Promise] returns fep', | ||
]); | ||
}); | ||
|
||
test('weakMap vref handling', async t => { | ||
insistGC(t); | ||
const log = []; | ||
const { | ||
VirtualObjectAwareWeakMap, | ||
VirtualObjectAwareWeakSet, | ||
valToSlot, | ||
slotToVal, | ||
} = makeFakeVirtualObjectManager(3, log); | ||
|
||
function addCListEntry(slot, val) { | ||
slotToVal.set(slot, val); | ||
valToSlot.set(val, slot); | ||
} | ||
|
||
function removeCListEntry(slot, val) { | ||
slotToVal.delete(slot); | ||
valToSlot.delete(val); | ||
} | ||
|
||
const weakMap = new VirtualObjectAwareWeakMap(); | ||
|
||
function checkMap(vref, label, useVRef) { | ||
const obj = {}; | ||
addCListEntry(vref, obj); | ||
weakMap.set(obj, label); | ||
t.is(weakMap.get(obj), label); | ||
removeCListEntry(vref, obj); | ||
const obj2 = {}; | ||
addCListEntry(vref, obj2); | ||
if (useVRef) { | ||
t.falsy(weakMap.has(obj)); | ||
t.truthy(weakMap.has(obj2)); | ||
t.is(weakMap.get(obj), undefined); | ||
t.is(weakMap.get(obj2), label); | ||
} else { | ||
t.truthy(weakMap.has(obj)); | ||
t.falsy(weakMap.has(obj2)); | ||
t.is(weakMap.get(obj), label); | ||
t.is(weakMap.get(obj2), undefined); | ||
} | ||
} | ||
|
||
checkMap('o-1', 'imported presence', true); | ||
checkMap('o+2', 'exported remotable', false); | ||
checkMap('o+3/4', 'exported virtual object', true); | ||
checkMap('p-5', 'imported promise', false); | ||
checkMap('p+6', 'exported promise', false); | ||
checkMap('d-7', 'imported device', false); | ||
|
||
FUDCo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const weakSet = new VirtualObjectAwareWeakSet(); | ||
|
||
function checkSet(vref, useVRef) { | ||
const obj = {}; | ||
addCListEntry(vref, obj); | ||
weakSet.add(obj); | ||
t.truthy(weakSet.has(obj)); | ||
removeCListEntry(vref, obj); | ||
const obj2 = {}; | ||
addCListEntry(vref, obj2); | ||
if (useVRef) { | ||
t.falsy(weakSet.has(obj)); | ||
t.truthy(weakSet.has(obj2)); | ||
} else { | ||
t.truthy(weakSet.has(obj)); | ||
t.falsy(weakSet.has(obj2)); | ||
} | ||
} | ||
|
||
checkSet('o-8', true); | ||
checkSet('o+9', false); | ||
checkSet('o+10/11', true); | ||
checkSet('p-12', false); | ||
checkSet('p+13', false); | ||
checkSet('d-14', false); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
/* global makeKind */ | ||
import { Far } from '@agoric/marshal'; | ||
|
||
function makeHolderInstance(state) { | ||
function init(value) { | ||
state.value = value; | ||
} | ||
const self = Far('holder-vo', { | ||
getValue() { | ||
return state.value; | ||
}, | ||
setValue(newValue) { | ||
state.value = newValue; | ||
}, | ||
}); | ||
return { init, self }; | ||
} | ||
|
||
const holderMaker = makeKind(makeHolderInstance); | ||
|
||
export function buildRootObject() { | ||
const testWeakMap = new WeakMap(); | ||
let memorableExportRemotable; | ||
let memorableExportVirtualObject; | ||
let memorableExportPromise; | ||
let forgetableExportRemotable; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be even more explicit, you could put the |
||
let forgetableExportVirtualObject; | ||
let forgetableExportPromise; | ||
|
||
return Far('root', { | ||
prepareWeakMap(theirStuff) { | ||
memorableExportRemotable = Far('remember-exp', {}); | ||
memorableExportVirtualObject = holderMaker('remember-exp-vo'); | ||
memorableExportPromise = new Promise((_res, _rej) => {}); | ||
forgetableExportRemotable = Far('forget-exp', {}); | ||
forgetableExportVirtualObject = holderMaker('forget-exp-vo'); | ||
forgetableExportPromise = new Promise((_res, _rej) => {}); | ||
|
||
const result = [ | ||
memorableExportRemotable, | ||
memorableExportVirtualObject, | ||
memorableExportPromise, | ||
forgetableExportRemotable, | ||
forgetableExportVirtualObject, | ||
forgetableExportPromise, | ||
]; | ||
|
||
let i = 0; | ||
for (const item of theirStuff) { | ||
testWeakMap.set(item, `imported item #${i}`); | ||
i += 1; | ||
} | ||
testWeakMap.set(memorableExportRemotable, 'mer'); | ||
testWeakMap.set(memorableExportVirtualObject, 'mevo'); | ||
testWeakMap.set(memorableExportPromise, 'mep'); | ||
testWeakMap.set(forgetableExportRemotable, 'fer'); | ||
testWeakMap.set(forgetableExportVirtualObject, 'fevo'); | ||
testWeakMap.set(forgetableExportPromise, 'fep'); | ||
forgetableExportRemotable = null; | ||
forgetableExportVirtualObject = null; | ||
forgetableExportPromise = null; | ||
|
||
return result; | ||
}, | ||
probeWeakMap(probe) { | ||
return testWeakMap.get(probe); | ||
}, | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great for addressing #3115: keys which are Presences or Representatives use their vref as the index, keys which are Remotables (or any non-vref-bearing object) use the
Object
identity.