From 0b4c0188b655e2490defe0db76b841def0d50484 Mon Sep 17 00:00:00 2001 From: Kris Kowal Date: Wed, 20 Jan 2021 20:59:49 -0800 Subject: [PATCH] test(xsnap): Test xsnap close and terminate --- packages/xsnap/src/netstring.js | 2 +- packages/xsnap/src/node-stream.js | 8 ++-- packages/xsnap/src/xsnap.js | 35 ++++++++------- packages/xsnap/test/test-xsnap.js | 73 +++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 22 deletions(-) diff --git a/packages/xsnap/src/netstring.js b/packages/xsnap/src/netstring.js index 2391c915f3e..8e830226aa5 100644 --- a/packages/xsnap/src/netstring.js +++ b/packages/xsnap/src/netstring.js @@ -17,7 +17,7 @@ const encoder = new TextEncoder(); * @param {AsyncIterable} input * @param {string=} name * @param {number=} capacity - * @returns {AsyncIterableIterator} input + * @returns {AsyncGenerator} input */ export async function* reader(input, name = '', capacity = 1024) { let length = 0; diff --git a/packages/xsnap/src/node-stream.js b/packages/xsnap/src/node-stream.js index e9dcf51c69b..2e109d9dfb5 100644 --- a/packages/xsnap/src/node-stream.js +++ b/packages/xsnap/src/node-stream.js @@ -21,10 +21,11 @@ const continues = { value: undefined }; * Back pressure emerges from awaiting on the promise * returned by `next` before calling `next` again. * - * @param {NodeJS.WritableStream} output + * @param {NodeJS.WritableStream} output the destination Node.js writer + * @param {string} [name] a debug name for stream errors * @returns {Stream} */ -export function writer(output) { +export function writer(output, name = '') { /** * @type {Deferred>} */ @@ -32,8 +33,7 @@ export function writer(output) { drained.resolve(continues); output.on('error', err => { - console.log('err', err); - drained.reject(err); + drained.reject(new Error(`Cannot write ${name}: ${err.message}`)); }); output.on('drain', () => { diff --git a/packages/xsnap/src/xsnap.js b/packages/xsnap/src/xsnap.js index bdc9c4f76fc..c307508a8a3 100644 --- a/packages/xsnap/src/xsnap.js +++ b/packages/xsnap/src/xsnap.js @@ -50,8 +50,8 @@ export function xsnap(options) { handleCommand = echoCommand, debug = false, snapshot = undefined, - stdout = 'inherit', - stderr = 'inherit', + stdout = 'ignore', + stderr = 'ignore', } = options; const platform = { @@ -64,12 +64,8 @@ export function xsnap(options) { throw new Error(`xsnap does not support platform ${os}`); } - const xsnapBin = new URL( - `../build/bin/${platform}/release/xsnap`, - importMetaUrl, - ).pathname; - const xsnapDebugBin = new URL( - `../build/bin/${platform}/debug/xsnap`, + const bin = new URL( + `../build/bin/${platform}/${debug ? 'debug' : 'release'}/xsnap`, importMetaUrl, ).pathname; @@ -78,15 +74,15 @@ export function xsnap(options) { const args = snapshot ? ['-r', snapshot] : []; - const bin = debug ? xsnapDebugBin : xsnapBin; - const xsnapProcess = spawn(bin, args, { stdio: ['ignore', stdout, stderr, 'pipe', 'pipe'], }); - xsnapProcess.on('exit', code => { - if (code === 0 || code === null) { + xsnapProcess.on('exit', (code, signal) => { + if (code === 0) { vatExit.resolve(); + } else if (signal !== null) { + vatExit.reject(new Error(`${name} exited due to signal ${signal}`)); } else { vatExit.reject(new Error(`${name} exited with code ${code}`)); } @@ -97,7 +93,10 @@ export function xsnap(options) { ); const messagesToXsnap = netstring.writer( - node.writer(/** @type {NodeJS.WritableStream} */ (xsnapProcess.stdio[3])), + node.writer( + /** @type {NodeJS.WritableStream} */ (xsnapProcess.stdio[3]), + `messages to ${name}`, + ), ); const messagesFromXsnap = netstring.reader( /** @type {AsyncIterable} */ (xsnapProcess.stdio[4]), @@ -114,7 +113,7 @@ export function xsnap(options) { const { done, value: message } = await messagesFromXsnap.next(); if (done) { xsnapProcess.kill(); - throw new Error('xsnap protocol error: unexpected end of output'); + return vatCancelled; } if (message.byteLength === 0) { // A protocol error kills the xsnap child process and breaks the baton @@ -214,8 +213,10 @@ export function xsnap(options) { * @returns {Promise} */ async function close() { - await messagesToXsnap.return(); - baton = Promise.reject(new Error(`xsnap closed`)); + baton = baton.then(async () => { + await messagesToXsnap.return(); + throw new Error(`${name} closed`); + }); baton.catch(() => {}); // Suppress Node.js unhandled exception warning. return vatExit.promise; } @@ -225,7 +226,7 @@ export function xsnap(options) { */ async function terminate() { xsnapProcess.kill(); - baton = Promise.reject(new Error(`xsnap closed`)); + baton = Promise.reject(new Error(`${name} terminated`)); baton.catch(() => {}); // Suppress Node.js unhandled exception warning. // Mute the vatExit exception: it is expected. return vatExit.promise.catch(() => {}); diff --git a/packages/xsnap/test/test-xsnap.js b/packages/xsnap/test/test-xsnap.js index d4b1c1c3f40..a8657efdf8b 100644 --- a/packages/xsnap/test/test-xsnap.js +++ b/packages/xsnap/test/test-xsnap.js @@ -9,8 +9,12 @@ const decoder = new TextDecoder(); const encoder = new TextEncoder(); const xsnapOptions = { + name: 'xsnap test worker', spawn: childProcess.spawn, os: os.type(), + stderr: 'inherit', + stdout: 'inherit', + debug: true }; test('evaluate and issueCommand', async t => { @@ -159,3 +163,72 @@ test('write and read snapshot', async t => { t.deepEqual(['Hello, World!'], messages); }); + +function delay(ms) { + return new Promise(resolve => setTimeout(resolve, ms)); +} + +test('fail to send command to already-closed xnsap worker', async t => { + const vat = xsnap({ ...xsnapOptions }); + await vat.close(); + await vat.evaluate(``).catch(err => { + t.is(err.message, 'xsnap test worker exited') + }); +}); + +test('fail to send command to already-terminated xnsap worker', async t => { + const vat = xsnap({ ...xsnapOptions }); + await vat.terminate(); + await vat.evaluate(``).catch(err => { + t.is(err.message, 'xsnap test worker exited due to signal SIGTERM') + }); +}); + +test('fail to send command to terminated xnsap worker', async t => { + const vat = xsnap({ ...xsnapOptions }); + const hang = vat.evaluate(`for (;;) {}`).then( + () => t.fail('command should not complete'), + err => { + t.is(err.message, 'Cannot write messages to xsnap test worker: write EPIPE') + }, + ); + + await vat.terminate(); + await hang; +}); + +test('abnormal termination', async t => { + const vat = xsnap({ ...xsnapOptions }); + const hang = vat.evaluate(`for (;;) {}`).then( + () => t.fail('command should not complete'), + err => { + t.is(err.message, 'xsnap test worker exited due to signal SIGTERM') + }, + ); + + // Allow the evaluate command to flush. + await delay(10); + await vat.terminate(); + await hang; +}); + +test('normal close of pathological script', async t => { + const vat = xsnap({ ...xsnapOptions }); + const hang = vat.evaluate(`for (;;) {}`).then( + () => t.fail('command should not complete'), + err => { + t.is(err.message, 'xsnap test worker exited due to signal SIGTERM') + }, + ); + // Allow the evaluate command to flush. + await delay(10); + // Close must timeout and the evaluation command + // must hang. + await Promise.race([ + vat.close().then(() => t.fail()), + hang, + delay(10), + ]); + await vat.terminate(); + await hang; +});