From 5583c305705604852ea32e0bbd6a500badd1777d Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Mon, 3 Apr 2023 15:35:46 -0400 Subject: [PATCH 1/7] wasi: make returnOnExit true by default Refs: https://github.com/nodejs/node/issues/46923 Signed-off-by: Michael Dawson --- doc/api/wasi.md | 12 ++++++++---- lib/wasi.js | 8 +++++--- test/wasi/test-wasi.js | 16 +++++++++++++++- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/doc/api/wasi.md b/doc/api/wasi.md index ad4597ab30dbc4..8f5c28d19b790b 100644 --- a/doc/api/wasi.md +++ b/doc/api/wasi.md @@ -121,6 +121,9 @@ changes: - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/47391 description: The version option is now required and has no default value. + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/47390 + description: default for returnOnExit changed to true. - version: v19.8.0 pr-url: https://github.com/nodejs/node/pull/46469 description: version field added to options. @@ -136,10 +139,11 @@ changes: sandbox directory structure. The string keys of `preopens` are treated as directories within the sandbox. The corresponding values in `preopens` are the real paths to those directories on the host machine. - * `returnOnExit` {boolean} By default, WASI applications terminate the Node.js - process via the `__wasi_proc_exit()` function. Setting this option to `true` - causes `wasi.start()` to return the exit code rather than terminate the - process. **Default:** `false`. + * `returnOnExit` {boolean} By default, when WASI applications call + `__wasi_proc_exit()` `wasi.start()` will returns with the exit code + specified rather than terminatng the process. Setting this option to + `false` will cause the Node.js process to exit with the specified + exit code instead. **Default:** `true`. * `stdin` {integer} The file descriptor used as standard input in the WebAssembly application. **Default:** `0`. * `stdout` {integer} The file descriptor used as standard output in the diff --git a/lib/wasi.js b/lib/wasi.js index e3d4294eeae743..a50164b2eb4b0b 100644 --- a/lib/wasi.js +++ b/lib/wasi.js @@ -102,11 +102,13 @@ class WASI { wrap[prop] = FunctionPrototypeBind(wrap[prop], wrap); } + let returnOnExit = true; if (options.returnOnExit !== undefined) { - validateBoolean(options.returnOnExit, 'options.returnOnExit'); - if (options.returnOnExit) - wrap.proc_exit = FunctionPrototypeBind(wasiReturnOnProcExit, this); + returnOnExit = options.returnOnExit; + validateBoolean(returnOnExit, 'options.returnOnExit'); } + if (returnOnExit === true) + wrap.proc_exit = FunctionPrototypeBind(wasiReturnOnProcExit, this); this[kSetMemory] = wrap._setMemory; delete wrap._setMemory; diff --git a/test/wasi/test-wasi.js b/test/wasi/test-wasi.js index cbfaf35d208b26..21e37b48b62196 100644 --- a/test/wasi/test-wasi.js +++ b/test/wasi/test-wasi.js @@ -1,6 +1,14 @@ 'use strict'; const common = require('../common'); +function returnOnExitEnvToValue(env) { + const envValue = env.RETURN_ON_EXIT; + if (envValue !== undefined) + return envValue === 'true'; + + return undefined; +} + if (process.argv[2] === 'wasi-child-preview1') { // Test version set to preview1 const assert = require('assert'); @@ -23,6 +31,7 @@ if (process.argv[2] === 'wasi-child-preview1') { '/sandbox': fixtures.path('wasi'), '/tmp': tmpdir.path, }, + returnOnExit: returnOnExitEnvToValue(process.env), }); // Validate the getImportObject helper @@ -56,6 +65,9 @@ if (process.argv[2] === 'wasi-child-preview1') { if (options.stdin !== undefined) opts.input = options.stdin; + if (options.returnOnExit === false) + opts.env.RETURN_ON_EXIT = 'false'; + const child = cp.spawnSync(process.execPath, [ ...args, __filename, @@ -79,7 +91,9 @@ if (process.argv[2] === 'wasi-child-preview1') { if (!common.isIBMi) { runWASI({ test: 'clock_getres' }); } - runWASI({ test: 'exitcode', exitCode: 120 }); + runWASI({ test: 'exitcode' }); + runWASI({ test: 'exitcode', returnOnExit: true }); + runWASI({ test: 'exitcode', exitCode: 120, returnOnExit: false }); runWASI({ test: 'fd_prestat_get_refresh' }); runWASI({ test: 'freopen', stdout: `hello from input2.txt${checkoutEOL}` }); runWASI({ test: 'ftruncate' }); From 35eb47ba8db5fff7029343bb4132e724bc5bc5fc Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 4 Apr 2023 17:04:35 -0400 Subject: [PATCH 2/7] Update doc/api/wasi.md Co-authored-by: Mohammed Keyvanzadeh --- doc/api/wasi.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/wasi.md b/doc/api/wasi.md index 8f5c28d19b790b..8efc496464096b 100644 --- a/doc/api/wasi.md +++ b/doc/api/wasi.md @@ -123,7 +123,7 @@ changes: description: The version option is now required and has no default value. - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/47390 - description: default for returnOnExit changed to true. + description: default value of returnOnExit changed to true. - version: v19.8.0 pr-url: https://github.com/nodejs/node/pull/46469 description: version field added to options. From ba8fc086741e2ad27804af3283d78acdf20ada6c Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 4 Apr 2023 17:04:42 -0400 Subject: [PATCH 3/7] Update doc/api/wasi.md Co-authored-by: Mohammed Keyvanzadeh --- doc/api/wasi.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/wasi.md b/doc/api/wasi.md index 8efc496464096b..eddc6a057e5d40 100644 --- a/doc/api/wasi.md +++ b/doc/api/wasi.md @@ -140,7 +140,7 @@ changes: directories within the sandbox. The corresponding values in `preopens` are the real paths to those directories on the host machine. * `returnOnExit` {boolean} By default, when WASI applications call - `__wasi_proc_exit()` `wasi.start()` will returns with the exit code + `__wasi_proc_exit()` `wasi.start()` will return with the exit code specified rather than terminatng the process. Setting this option to `false` will cause the Node.js process to exit with the specified exit code instead. **Default:** `true`. From 76470cabdb3cb18b0453ea385bd25c4dbe3480da Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 4 Apr 2023 17:05:40 -0400 Subject: [PATCH 4/7] Update test/wasi/test-wasi.js Co-authored-by: Colin Ihrig --- test/wasi/test-wasi.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/wasi/test-wasi.js b/test/wasi/test-wasi.js index 21e37b48b62196..7ac69398ddf5b5 100644 --- a/test/wasi/test-wasi.js +++ b/test/wasi/test-wasi.js @@ -3,10 +3,11 @@ const common = require('../common'); function returnOnExitEnvToValue(env) { const envValue = env.RETURN_ON_EXIT; - if (envValue !== undefined) - return envValue === 'true'; + if (envValue === undefined) { + return undefined; + } - return undefined; + return envValue === 'true'; } if (process.argv[2] === 'wasi-child-preview1') { From 99d8a2f8d4013bbedbfce5b1c37b9a86a54b9dd4 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 4 Apr 2023 17:06:04 -0400 Subject: [PATCH 5/7] Update lib/wasi.js Co-authored-by: Mohammed Keyvanzadeh --- lib/wasi.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/wasi.js b/lib/wasi.js index a50164b2eb4b0b..3d6cd6feb504d9 100644 --- a/lib/wasi.js +++ b/lib/wasi.js @@ -107,7 +107,7 @@ class WASI { returnOnExit = options.returnOnExit; validateBoolean(returnOnExit, 'options.returnOnExit'); } - if (returnOnExit === true) + if (returnOnExit) wrap.proc_exit = FunctionPrototypeBind(wasiReturnOnProcExit, this); this[kSetMemory] = wrap._setMemory; From 1437defa510587636df62e1ea38e08e994fcc588 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 4 Apr 2023 17:06:22 -0400 Subject: [PATCH 6/7] Update lib/wasi.js Co-authored-by: Mohammed Keyvanzadeh --- lib/wasi.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/wasi.js b/lib/wasi.js index 3d6cd6feb504d9..6c700219a80d4b 100644 --- a/lib/wasi.js +++ b/lib/wasi.js @@ -104,8 +104,8 @@ class WASI { let returnOnExit = true; if (options.returnOnExit !== undefined) { + validateBoolean(options.returnOnExit, 'options.returnOnExit'); returnOnExit = options.returnOnExit; - validateBoolean(returnOnExit, 'options.returnOnExit'); } if (returnOnExit) wrap.proc_exit = FunctionPrototypeBind(wasiReturnOnProcExit, this); From 773abe59ee6e1af67d5019119d0b28fa1dbd2a68 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 4 Apr 2023 17:24:46 -0400 Subject: [PATCH 7/7] squash: address comments Signed-off-by: Michael Dawson --- test/wasi/test-wasi.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/wasi/test-wasi.js b/test/wasi/test-wasi.js index 7ac69398ddf5b5..f1f3ece4b4e1a6 100644 --- a/test/wasi/test-wasi.js +++ b/test/wasi/test-wasi.js @@ -66,8 +66,9 @@ if (process.argv[2] === 'wasi-child-preview1') { if (options.stdin !== undefined) opts.input = options.stdin; - if (options.returnOnExit === false) - opts.env.RETURN_ON_EXIT = 'false'; + if ('returnOnExit' in options) { + opts.env.RETURN_ON_EXIT = options.returnOnExit; + } const child = cp.spawnSync(process.execPath, [ ...args,