From 80ebb4282d29733fefbcee8deb9cc70348eace16 Mon Sep 17 00:00:00 2001 From: Jared Kantrowitz Date: Sat, 24 Jun 2017 15:37:59 -0400 Subject: [PATCH] src: adjust windows abort behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Raising SIGABRT is handled in the CRT in windows, calling _exit() with ambiguous code "3" by default. This adjustment to the abort behavior gives a more sane exit code on abort, by calling _exit directly with code 134. PR-URL: https://github.com/nodejs/node/pull/13947 Fixes: https://github.com/nodejs/node/issues/12271 Refs: https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/abort Reviewed-By: Refael Ackermann Reviewed-By: Richard Lau Reviewed-By: Timothy Gu Reviewed-By: Tobias Nießen Reviewed-By: James M Snell Reviewed-By: Gireesh Punathil Reviewed-By: Michael Dawson --- doc/api/process.md | 2 ++ src/util.h | 2 +- test/abort/test-abort-uncaught-exception.js | 2 +- test/async-hooks/test-callback-error.js | 2 +- test/common/index.js | 6 ++--- test/parallel/test-windows-abort-exitcode.js | 2 +- .../test-windows-failed-heap-allocation.js | 24 +++++++++++++++++++ 7 files changed, 33 insertions(+), 7 deletions(-) create mode 100644 test/parallel/test-windows-failed-heap-allocation.js diff --git a/doc/api/process.md b/doc/api/process.md index 2a85a551c9..d723f9721b 100644 --- a/doc/api/process.md +++ b/doc/api/process.md @@ -1819,6 +1819,8 @@ cases: value of the signal code. This is a standard Unix practice, since exit codes are defined to be 7-bit integers, and signal exits set the high-order bit, and then contain the value of the signal code. + For example, signal `SIGABRT` has value `6`, so the expected exit + code will be `128` + `6`, or `134`. [`'exit'`]: #process_event_exit diff --git a/src/util.h b/src/util.h index 1272de1893..08308d837f 100644 --- a/src/util.h +++ b/src/util.h @@ -95,7 +95,7 @@ template using remove_reference = std::remove_reference; // Windows 8+ does not like abort() in Release mode #ifdef _WIN32 -#define ABORT_NO_BACKTRACE() raise(SIGABRT) +#define ABORT_NO_BACKTRACE() _exit(134) #else #define ABORT_NO_BACKTRACE() abort() #endif diff --git a/test/abort/test-abort-uncaught-exception.js b/test/abort/test-abort-uncaught-exception.js index f984719376..fb30cfba3c 100644 --- a/test/abort/test-abort-uncaught-exception.js +++ b/test/abort/test-abort-uncaught-exception.js @@ -21,7 +21,7 @@ function run(flags, signals) { child.on('exit', common.mustCall(function(code, sig) { if (common.isWindows) { if (signals) - assert.strictEqual(code, 3); + assert.strictEqual(code, 0xC0000005); else assert.strictEqual(code, 1); } else { diff --git a/test/async-hooks/test-callback-error.js b/test/async-hooks/test-callback-error.js index a4b8a99f33..b2093f74e2 100644 --- a/test/async-hooks/test-callback-error.js +++ b/test/async-hooks/test-callback-error.js @@ -101,7 +101,7 @@ assert.ok(!arg); child.on('close', (code, signal) => { clearTimeout(tO); if (common.isWindows) { - assert.strictEqual(code, 3); + assert.strictEqual(code, 134); assert.strictEqual(signal, null); } else { assert.strictEqual(code, null); diff --git a/test/common/index.js b/test/common/index.js index 9358f86f65..54742319d2 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -605,10 +605,10 @@ exports.nodeProcessAborted = function nodeProcessAborted(exitCode, signal) { // On Windows, 'aborts' are of 2 types, depending on the context: // (i) Forced access violation, if --abort-on-uncaught-exception is on // which corresponds to exit code 3221225477 (0xC0000005) - // (ii) raise(SIGABRT) or abort(), which lands up in CRT library calls - // which corresponds to exit code 3. + // (ii) Otherwise, _exit(134) which is called in place of abort() due to + // raising SIGABRT exiting with ambiguous exit code '3' by default if (exports.isWindows) - expectedExitCodes = [3221225477, 3]; + expectedExitCodes = [0xC0000005, 134]; // When using --abort-on-uncaught-exception, V8 will use // base::OS::Abort to terminate the process. diff --git a/test/parallel/test-windows-abort-exitcode.js b/test/parallel/test-windows-abort-exitcode.js index d61a91315b..e2e6570c1c 100644 --- a/test/parallel/test-windows-abort-exitcode.js +++ b/test/parallel/test-windows-abort-exitcode.js @@ -16,7 +16,7 @@ if (process.argv[2] === 'child') { } else { const child = spawn(process.execPath, [__filename, 'child']); child.on('exit', common.mustCall((code, signal) => { - assert.strictEqual(code, 3); + assert.strictEqual(code, 134); assert.strictEqual(signal, null); })); } diff --git a/test/parallel/test-windows-failed-heap-allocation.js b/test/parallel/test-windows-failed-heap-allocation.js new file mode 100644 index 0000000000..465dcae2b6 --- /dev/null +++ b/test/parallel/test-windows-failed-heap-allocation.js @@ -0,0 +1,24 @@ +'use strict'; +const common = require('../common'); + +// This test ensures that an out of memory error exits with code 134 on Windows + +if (!common.isWindows) return common.skip('Windows-only'); + +const assert = require('assert'); +const { exec } = require('child_process'); + +if (process.argv[2] === 'heapBomb') { + // heap bomb, imitates a memory leak quickly + const fn = (nM) => [...Array(nM)].map((i) => fn(nM ** 2)); + fn(2); +} + +// --max-old-space-size=3 is the min 'old space' in V8, explodes fast +const cmd = `"${process.execPath}" --max-old-space-size=3 "${__filename}"`; +exec(`${cmd} heapBomb`, common.mustCall((err) => { + const msg = `Wrong exit code of ${err.code}! Expected 134 for abort`; + // Note: common.nodeProcessAborted() is not asserted here because it + // returns true on 134 as well as 0xC0000005 (V8's base::OS::Abort) + assert.strictEqual(err.code, 134, msg); +}));