Skip to content

Commit

Permalink
src: adjust windows abort behavior
Browse files Browse the repository at this point in the history
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: #13947
Fixes: #12271
Refs: https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/abort
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
  • Loading branch information
jkantr authored and refack committed Aug 8, 2017
1 parent 7535a94 commit 80ebb42
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 7 deletions.
2 changes: 2 additions & 0 deletions doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ template <typename T> using remove_reference = std::remove_reference<T>;

// 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
Expand Down
2 changes: 1 addition & 1 deletion test/abort/test-abort-uncaught-exception.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion test/async-hooks/test-callback-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-windows-abort-exitcode.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}));
}
24 changes: 24 additions & 0 deletions test/parallel/test-windows-failed-heap-allocation.js
Original file line number Diff line number Diff line change
@@ -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);
}));

0 comments on commit 80ebb42

Please sign in to comment.