From 9e6e31ac55521893b6fdf31785bb901345ed46af Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Wed, 16 Sep 2020 12:28:39 -0700 Subject: [PATCH] fix: handle syscallResult and deliveryResult consistently among workers fixes #1775 --- .../src/kernel/vatManager/nodeWorker.js | 9 ++++++- .../kernel/vatManager/nodeWorkerSupervisor.js | 22 +++++++++------- .../kernel/vatManager/subprocessSupervisor.js | 22 +++++++++------- .../vatManager/worker-subprocess-node.js | 11 +++++++- packages/xs-vat-worker/src/vatWorker.js | 26 +++++++++++-------- 5 files changed, 57 insertions(+), 33 deletions(-) diff --git a/packages/SwingSet/src/kernel/vatManager/nodeWorker.js b/packages/SwingSet/src/kernel/vatManager/nodeWorker.js index 7e249102cd1..2f631d1dd5c 100644 --- a/packages/SwingSet/src/kernel/vatManager/nodeWorker.js +++ b/packages/SwingSet/src/kernel/vatManager/nodeWorker.js @@ -55,10 +55,16 @@ export function makeNodeWorkerVatManagerFactory(tools) { transcriptManager, ); function handleSyscall(vatSyscallObject) { + // we are invoked by an async postMessage from the worker thread, whose + // vat code has moved on (it really wants a synchronous/immediate + // syscall) const type = vatSyscallObject[0]; if (type === 'callNow') { throw Error(`nodeWorker cannot block, cannot use syscall.callNow`); } + // This might throw an Error if the syscall was faulty, in which case + // the vat will be terminated soon. It returns a vatSyscallResults, + // which we discard because there is nobody to send it to. doSyscall(vatSyscallObject); } @@ -96,7 +102,8 @@ export function makeNodeWorkerVatManagerFactory(tools) { if (waiting) { const resolve = waiting; waiting = null; - resolve(); + const deliveryResult = args; + resolve(deliveryResult); } } else { parentLog(`unrecognized uplink message ${type}`); diff --git a/packages/SwingSet/src/kernel/vatManager/nodeWorkerSupervisor.js b/packages/SwingSet/src/kernel/vatManager/nodeWorkerSupervisor.js index 705d78fe11d..b1e9955ca1b 100644 --- a/packages/SwingSet/src/kernel/vatManager/nodeWorkerSupervisor.js +++ b/packages/SwingSet/src/kernel/vatManager/nodeWorkerSupervisor.js @@ -46,6 +46,16 @@ async function doProcess(dispatchRecord, errmsg) { workerLog(`runAndWait`); await runAndWait(() => dispatch[dispatchOp](...dispatchArgs), errmsg); workerLog(`doProcess done`); + const vatDeliveryResults = harden(['ok']); + return vatDeliveryResults; +} + +function doMessage(targetSlot, msg) { + const errmsg = `vat[${targetSlot}].${msg.method} dispatch failed`; + return doProcess( + ['deliver', targetSlot, msg.method, msg.args, msg.result], + errmsg, + ); } function doNotify(vpid, vp) { @@ -65,7 +75,6 @@ function doNotify(vpid, vp) { } } -let syscallLog; parentPort.on('message', ([type, ...margs]) => { workerLog(`received`, type); if (type === 'start') { @@ -121,16 +130,9 @@ parentPort.on('message', ([type, ...margs]) => { } const [dtype, ...dargs] = margs; if (dtype === 'message') { - const [targetSlot, msg] = dargs; - const errmsg = `vat[${targetSlot}].${msg.method} dispatch failed`; - doProcess( - ['deliver', targetSlot, msg.method, msg.args, msg.result], - errmsg, - ).then(() => { - sendUplink(['deliverDone']); - }); + doMessage(...dargs).then(res => sendUplink(['deliverDone', ...res])); } else if (dtype === 'notify') { - doNotify(...dargs).then(() => sendUplink(['deliverDone', syscallLog])); + doNotify(...dargs).then(res => sendUplink(['deliverDone', ...res])); } else { throw Error(`bad delivery type ${dtype}`); } diff --git a/packages/SwingSet/src/kernel/vatManager/subprocessSupervisor.js b/packages/SwingSet/src/kernel/vatManager/subprocessSupervisor.js index 52c65209ab9..5a8c71abc85 100644 --- a/packages/SwingSet/src/kernel/vatManager/subprocessSupervisor.js +++ b/packages/SwingSet/src/kernel/vatManager/subprocessSupervisor.js @@ -42,6 +42,16 @@ async function doProcess(dispatchRecord, errmsg) { workerLog(`runAndWait`); await runAndWait(() => dispatch[dispatchOp](...dispatchArgs), errmsg); workerLog(`doProcess done`); + const vatDeliveryResults = harden(['ok']); + return vatDeliveryResults; +} + +function doMessage(targetSlot, msg) { + const errmsg = `vat[${targetSlot}].${msg.method} dispatch failed`; + return doProcess( + ['deliver', targetSlot, msg.method, msg.args, msg.result], + errmsg, + ); } function doNotify(vpid, vp) { @@ -79,7 +89,6 @@ function sendUplink(msg) { // toParent.write('child ack'); // }); -let syscallLog; fromParent.on('data', data => { const [type, ...margs] = JSON.parse(data); workerLog(`received`, type); @@ -136,16 +145,9 @@ fromParent.on('data', data => { } const [dtype, ...dargs] = margs; if (dtype === 'message') { - const [targetSlot, msg] = dargs; - const errmsg = `vat[${targetSlot}].${msg.method} dispatch failed`; - doProcess( - ['deliver', targetSlot, msg.method, msg.args, msg.result], - errmsg, - ).then(() => { - sendUplink(['deliverDone']); - }); + doMessage(...dargs).then(res => sendUplink(['deliverDone', ...res])); } else if (dtype === 'notify') { - doNotify(...dargs).then(() => sendUplink(['deliverDone', syscallLog])); + doNotify(...dargs).then(res => sendUplink(['deliverDone', ...res])); } else { throw Error(`bad delivery type ${dtype}`); } diff --git a/packages/SwingSet/src/kernel/vatManager/worker-subprocess-node.js b/packages/SwingSet/src/kernel/vatManager/worker-subprocess-node.js index 056e8e05f55..90c4ebcf68d 100644 --- a/packages/SwingSet/src/kernel/vatManager/worker-subprocess-node.js +++ b/packages/SwingSet/src/kernel/vatManager/worker-subprocess-node.js @@ -56,10 +56,18 @@ export function makeNodeSubprocessFactory(tools) { transcriptManager, ); function handleSyscall(vatSyscallObject) { + // We are currently invoked by an async piped from the worker thread, + // whose vat code has moved on (it really wants a synchronous/immediate + // syscall). TODO: unlike threads, subprocesses could be made to wait + // by doing a blocking read from the pipe, so we could fix this, and + // re-enable syscall.callNow const type = vatSyscallObject[0]; if (type === 'callNow') { throw Error(`nodeWorker cannot block, cannot use syscall.callNow`); } + // This might throw an Error if the syscall was faulty, in which case + // the vat will be terminated soon. It returns a vatSyscallResults, + // which we discard because there is currently nobody to send it to. doSyscall(vatSyscallObject); } @@ -96,7 +104,8 @@ export function makeNodeSubprocessFactory(tools) { if (waiting) { const resolve = waiting; waiting = null; - resolve(); + const deliveryResult = args; + resolve(deliveryResult); } } else { parentLog(`unrecognized uplink message ${type}`); diff --git a/packages/xs-vat-worker/src/vatWorker.js b/packages/xs-vat-worker/src/vatWorker.js index cfdfc58a441..7933115f8c2 100644 --- a/packages/xs-vat-worker/src/vatWorker.js +++ b/packages/xs-vat-worker/src/vatWorker.js @@ -54,6 +54,16 @@ function makeWorker(io, setImmediate) { setImmediate, ); workerLog(`doProcess done`); + const vatDeliveryResults = harden(['ok']); + return vatDeliveryResults; + } + + function doMessage(targetSlot, msg) { + const errmsg = `vat[${targetSlot}].${msg.method} dispatch failed`; + return doProcess( + ['deliver', targetSlot, msg.method, msg.args, msg.result], + errmsg, + ); } function doNotify(vpid, vp) { @@ -82,7 +92,6 @@ function makeWorker(io, setImmediate) { // toParent.write('child ack'); // }); - let syscallLog; const handle = harden(async ([type, ...margs]) => { workerLog(`received`, type); if (type === 'start') { @@ -144,17 +153,12 @@ function makeWorker(io, setImmediate) { } const [dtype, ...dargs] = margs; if (dtype === 'message') { - const [targetSlot, msg] = dargs; - const errmsg = `vat[${targetSlot}].${msg.method} dispatch failed`; - await doProcess( - ['deliver', targetSlot, msg.method, msg.args, msg.result], - errmsg, - ).then(() => { - sendUplink(['deliverDone']); - }); + await doMessage(...dargs).then(res => + sendUplink(['deliverDone', ...res]), + ); } else if (dtype === 'notify') { - await doNotify(...dargs).then(() => - sendUplink(['deliverDone', syscallLog]), + await doNotify(...dargs).then(res => + sendUplink(['deliverDone', ...res]), ); } else { throw Error(`bad delivery type ${dtype}`);