process: add flag for uncaught exception abort

Introduce `process.shouldAbortOnUncaughtException` to control
`--abort-on-uncaught-exception` behaviour, and implement
some of the domains functionality on top of it.

Refs: nodejs#17143

doc/api/cli.md

@@ -183,6 +183,9 @@ added: v0.10
 Aborting instead of exiting causes a core file to be generated for post-mortem
 analysis using a debugger (such as `lldb`, `gdb`, and `mdb`).
 
+*Note*: If this flag is passed, the behavior can still be set to not abort
+through [`process.shouldAbortOnUncaughtException`][].
+
 ### `--trace-warnings`
 <!-- YAML
 added: v6.0.0
@@ -598,3 +601,4 @@ greater than `4` (its current default value).  For more information, see the
 [debugger]: debugger.html
 [emit_warning]: process.html#process_process_emitwarning_warning_type_code_ctor
 [libuv threadpool documentation]: http://docs.libuv.org/en/latest/threadpool.html
+[`process.shouldAbortOnUncaughtException`]: process.html#process_process_shouldabortonuncaughtexception

doc/api/process.md

@@ -1637,6 +1637,29 @@ if (process.getuid && process.setuid) {
 or Android).
 
 
+## process.shouldAbortOnUncaughtException
+<!-- YAML
+added: REPLACEME
+-->
+
+* {boolean} **Default:** `true`
+
+The `process.shouldAbortOnUncaughtException` switch controls the behavior
+when the `--abort-on-uncaught-exception` flag was passed on the command line
+or set through [`v8.setFlagsFromString()`][]:
+
+If the flag was passed to Node.js and `process.shouldAbortOnUncaughtException`
+is `true`, the process will abort when enountering an uncaught exception
+(regardless of any [`process.on('uncaughtException')`][] listeners).
+
+If the flag was passed to Node and `process.shouldAbortOnUncaughtException`
+is `false`, the process will not abort.
+
+If the `--abort-on-uncaught-exception` flag is not set, this value is ignored.
+
+*Note*: If the deprecated [`domain`][] built-in module is in use,
+this flag will be set by that module whenever domain state changes.
+
 ## process.stderr
 
 * {Stream}
@@ -1921,6 +1944,7 @@ cases:
 [`JSON.stringify` spec]: https://tc39.github.io/ecma262/#sec-json.stringify
 [`console.error()`]: console.html#console_console_error_data_args
 [`console.log()`]: console.html#console_console_log_data_args
+[`domain`]: domain.html
 [`end()`]: stream.html#stream_writable_end_chunk_encoding_callback
 [`net.Server`]: net.html#net_class_net_server
 [`net.Socket`]: net.html#net_class_net_socket
@@ -1930,11 +1954,13 @@ cases:
 [`process.exit()`]: #process_process_exit_code
 [`process.exitCode`]: #process_process_exitcode
 [`process.kill()`]: #process_process_kill_pid_signal
+[`process.on('uncaughtException')`]: process.html#process_event_uncaughtexception
 [`promise.catch()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/catch
 [`require()`]: globals.html#globals_require
 [`require.main`]: modules.html#modules_accessing_the_main_module
 [`require.resolve()`]: modules.html#modules_require_resolve_request_options
 [`setTimeout(fn, 0)`]: timers.html#timers_settimeout_callback_delay_args
+[`v8.setFlagsFromString()`]: v8.html#v8_v8_setflagsfromstring_flags
 [Child Process]: child_process.html
 [Cluster]: cluster.html
 [Duplex]: stream.html#stream_duplex_and_transform_streams

lib/domain.js

@@ -85,15 +85,22 @@ const asyncHook = createHook({
 // another one. The stack is each entered domain.
 const stack = [];
 exports._stack = stack;
-process._setupDomainUse(stack);
+process._setupDomainUse();
 
-class Domain extends EventEmitter {
+function updateShouldAbort() {
+  process.shouldAbortOnUncaughtException =
+      stack.every((domain) => domain.listenerCount('error') === 0);
+}
 
+class Domain extends EventEmitter {
   constructor() {
     super();
 
     this.members = [];
     asyncHook.enable();
+
+    this.on('removeListener', updateShouldAbort);
+    this.on('newListener', updateShouldAbort);
   }
 }
 
@@ -131,14 +138,15 @@ Domain.prototype._errorHandler = function _errorHandler(er) {
     // prevent the process 'uncaughtException' event from being emitted
     // if a listener is set.
     if (EventEmitter.listenerCount(this, 'error') > 0) {
+      const prevShouldAbort = process.shouldAbortOnUncaughtException;
       try {
-        // Set the _emittingTopLevelDomainError so that we know that, even
+        // Set shouldAbortOnUncaughtException so that we know that, even
         // if technically the top-level domain is still active, it would
         // be ok to abort on an uncaught exception at this point
-        process._emittingTopLevelDomainError = true;
+        process.shouldAbortOnUncaughtException = true;
         caught = this.emit('error', er);
       } finally {
-        process._emittingTopLevelDomainError = false;
+        process.shouldAbortOnUncaughtException = prevShouldAbort;
       }
     }
   } else {
@@ -185,6 +193,7 @@ Domain.prototype.enter = function() {
   // to push it onto the stack so that we can pop it later.
   exports.active = process.domain = this;
   stack.push(this);
+  updateShouldAbort();
 };
 
 
@@ -198,6 +207,7 @@ Domain.prototype.exit = function() {
 
   exports.active = stack[stack.length - 1];
   process.domain = exports.active;
+  updateShouldAbort();
 };
 
 

lib/internal/bootstrap_node.js

@@ -413,6 +413,23 @@
 
       return caught;
     };
+
+    // This is a typed array for faster communication with JS.
+    const shouldAbortOnUncaughtToggle = process._shouldAbortOnUncaughtToggle;
+    delete process._shouldAbortOnUncaughtToggle;
+    Object.defineProperty(process, 'shouldAbortOnUncaughtException', {
+      get() {
+        return !!shouldAbortOnUncaughtToggle[0];
+      },
+      set(value) {
+        shouldAbortOnUncaughtToggle[0] = +!!value;
+      },
+      // This should look like a normal property. In particular,
+      // userland modules should be able to replace it with a wrapper
+      // if necessary.
+      configurable: true,
+      enumerable: true
+    });
   }
 
   function setupV8() {

src/env-inl.h

@@ -268,6 +268,7 @@ inline Environment::Environment(IsolateData* isolate_data,
       emit_napi_warning_(true),
       makecallback_cntr_(0),
       scheduled_immediate_count_(isolate_, 1),
+      should_abort_on_uncaught_toggle_(isolate_, 1),
 #if HAVE_INSPECTOR
       inspector_agent_(new inspector::Agent(this)),
 #endif
@@ -305,6 +306,9 @@ inline Environment::Environment(IsolateData* isolate_data,
   performance_state_->milestones[
     performance::NODE_PERFORMANCE_MILESTONE_V8_START] =
         performance::performance_v8_start;
+
+  // By default, always abort when --abort-on-uncaught-exception was passed.
+  should_abort_on_uncaught_toggle_[0] = 1;
 }
 
 inline Environment::~Environment() {
@@ -399,6 +403,11 @@ inline void Environment::set_abort_on_uncaught_exception(bool value) {
   abort_on_uncaught_exception_ = value;
 }
 
+inline AliasedBuffer<uint32_t, v8::Uint32Array>&
+Environment::should_abort_on_uncaught_toggle() {
+  return should_abort_on_uncaught_toggle_;
+}
+
 inline std::vector<double>* Environment::destroy_async_id_list() {
   return &destroy_async_id_list_;
 }

src/env.h

@@ -134,7 +134,6 @@ class ModuleWrap;
   V(dns_txt_string, "TXT")                                                    \
   V(domain_string, "domain")                                                  \
   V(emit_string, "emit")                                                      \
-  V(emitting_top_level_domain_error_string, "_emittingTopLevelDomainError")   \
   V(exchange_string, "exchange")                                              \
   V(enumerable_string, "enumerable")                                          \
   V(idle_string, "idle")                                                      \
@@ -309,7 +308,6 @@ class ModuleWrap;
   V(internal_binding_cache_object, v8::Object)                                \
   V(buffer_prototype_object, v8::Object)                                      \
   V(context, v8::Context)                                                     \
-  V(domains_stack_array, v8::Array)                                           \
   V(http2ping_constructor_template, v8::ObjectTemplate)                       \
   V(http2stream_constructor_template, v8::ObjectTemplate)                     \
   V(inspector_console_api_object, v8::Object)                                 \
@@ -568,8 +566,15 @@ class Environment {
   void PrintSyncTrace() const;
   inline void set_trace_sync_io(bool value);
 
+  // This stores whether the --abort-on-uncaught-exception flag was passed
+  // to Node.
   inline bool abort_on_uncaught_exception() const;
   inline void set_abort_on_uncaught_exception(bool value);
+  // This is a pseudo-boolean that keeps track of whether an uncaught exception
+  // should abort the process or not if --abort-on-uncaught-exception was
+  // passed to Node. If the flag was not passed, it is ignored.
+  inline AliasedBuffer<uint32_t, v8::Uint32Array>&
+  should_abort_on_uncaught_toggle();
 
   // The necessary API for async_hooks.
   inline double new_async_id();
@@ -713,6 +718,7 @@ class Environment {
   std::vector<double> destroy_async_id_list_;
 
   AliasedBuffer<uint32_t, v8::Uint32Array> scheduled_immediate_count_;
+  AliasedBuffer<uint32_t, v8::Uint32Array> should_abort_on_uncaught_toggle_;
 
   performance::performance_state* performance_state_ = nullptr;
   std::map<std::string, uint64_t> performance_marks_;

src/node.cc

@@ -779,66 +779,13 @@ void* ArrayBufferAllocator::Allocate(size_t size) {
 
 namespace {
 
-bool DomainHasErrorHandler(const Environment* env,
-                           const Local<Object>& domain) {
-  HandleScope scope(env->isolate());
-
-  Local<Value> domain_event_listeners_v = domain->Get(env->events_string());
-  if (!domain_event_listeners_v->IsObject())
-    return false;
-
-  Local<Object> domain_event_listeners_o =
-      domain_event_listeners_v.As<Object>();
-
-  Local<Value> domain_error_listeners_v =
-      domain_event_listeners_o->Get(env->error_string());
-
-  if (domain_error_listeners_v->IsFunction() ||
-      (domain_error_listeners_v->IsArray() &&
-      domain_error_listeners_v.As<Array>()->Length() > 0))
-    return true;
-
-  return false;
-}
-
-bool DomainsStackHasErrorHandler(const Environment* env) {
-  HandleScope scope(env->isolate());
-
-  if (!env->using_domains())
-    return false;
-
-  Local<Array> domains_stack_array = env->domains_stack_array().As<Array>();
-  if (domains_stack_array->Length() == 0)
-    return false;
-
-  uint32_t domains_stack_length = domains_stack_array->Length();
-  for (uint32_t i = domains_stack_length; i > 0; --i) {
-    Local<Value> domain_v = domains_stack_array->Get(i - 1);
-    if (!domain_v->IsObject())
-      return false;
-
-    Local<Object> domain = domain_v.As<Object>();
-    if (DomainHasErrorHandler(env, domain))
-      return true;
-  }
-
-  return false;
-}
-
-
 bool ShouldAbortOnUncaughtException(Isolate* isolate) {
   HandleScope scope(isolate);
-
   Environment* env = Environment::GetCurrent(isolate);
-  Local<Object> process_object = env->process_object();
-  Local<String> emitting_top_level_domain_error_key =
-    env->emitting_top_level_domain_error_string();
-  bool isEmittingTopLevelDomainError =
-      process_object->Get(emitting_top_level_domain_error_key)->BooleanValue();
-
-  return isEmittingTopLevelDomainError || !DomainsStackHasErrorHandler(env);
+  return env->should_abort_on_uncaught_toggle()[0];
 }
 
+
 Local<Value> GetDomainProperty(Environment* env, Local<Object> object) {
   Local<Value> domain_v =
       object->GetPrivate(env->context(), env->domain_private_symbol())
@@ -888,9 +835,6 @@ void SetupDomainUse(const FunctionCallbackInfo<Value>& args) {
 
   HandleScope scope(env->isolate());
 
-  CHECK(args[0]->IsArray());
-  env->set_domains_stack_array(args[0].As<Array>());
-
   // Do a little housekeeping.
   env->process_object()->Delete(
       env->context(),
@@ -3168,6 +3112,13 @@ void SetupProcessObject(Environment* env,
                      scheduled_immediate_count,
                      env->scheduled_immediate_count().GetJSArray()).FromJust());
 
+  auto should_abort_on_uncaught_toggle =
+      FIXED_ONE_BYTE_STRING(env->isolate(), "_shouldAbortOnUncaughtToggle");
+  CHECK(process->Set(env->context(),
+                     should_abort_on_uncaught_toggle,
+                     env->should_abort_on_uncaught_toggle().GetJSArray())
+                         .FromJust());
+
   // -e, --eval
   if (eval_string) {
     READONLY_PROPERTY(process,

test/parallel/test-process-should-abort-on-uncaught-setflagsfromstring.js

@@ -0,0 +1,12 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const v8 = require('v8');
+
+assert.strictEqual(process.shouldAbortOnUncaughtException, true);
+
+v8.setFlagsFromString('--abort-on-uncaught-exception');
+// This should make the process not crash even though the flag was passed.
+process.shouldAbortOnUncaughtException = false;
+process.once('uncaughtException', common.mustCall());
+throw new Error('foo');

test/parallel/test-process-should-abort-on-uncaught.js

@@ -0,0 +1,11 @@
+// Flags: --abort-on-uncaught-exception
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+
+assert.strictEqual(process.shouldAbortOnUncaughtException, true);
+
+// This should make the process not crash even though the flag was passed.
+process.shouldAbortOnUncaughtException = false;
+process.once('uncaughtException', common.mustCall());
+throw new Error('foo');

test/parallel/test-process-uncaught-exception.js

@@ -0,0 +1,8 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+
+// This value is ignored when the --abort-on-uncaught-exception flag is missing.
+assert.strictEqual(process.shouldAbortOnUncaughtException, true);
+process.once('uncaughtException', common.mustCall());
+throw new Error('foo');