Skip to content

lib/src: exit on gc unhandled promise #15126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
BridgeAR wants to merge 6 commits into from
Closed

lib/src: exit on gc unhandled promise #15126

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
711d699
lib,src: throw on unhanded promise rejections
Fishrock123 Apr 25, 2016
af11384
lib: follow up to exit early
BridgeAR Aug 31, 2017
19c7eb3
[squash] fix linter errors
BridgeAR Sep 1, 2017
69bfdd6
[squash] fix a test and remove unnecessary fn
BridgeAR Sep 1, 2017
5d2c83e
[squash] address comments
BridgeAR Sep 26, 2017
8fd484e
[squash] fix test
BridgeAR Sep 26, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
lib,src: throw on unhanded promise rejections 
src: use std::map for the promise reject map

Refs: #5292
Refs: nodejs/promises#26
Refs: #6355
Refs: #6375
  • Loading branch information
@Fishrock123 @BridgeAR
Fishrock123 authored and BridgeAR committed Sep 26, 2017
commit 711d699d249c9d44859a45dc6ce54844dc8ed386
4 changes: 2 additions & 2 deletions lib/internal/bootstrap_node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@
const { clearIdStack, asyncIdStackSize } = async_wrap;
const { kAfter, kCurrentAsyncId, kInitTriggerId } = async_wrap.constants;

process._fatalException = function(er) {
process._fatalException = function(er, fromPromise) {
var caught;

// It's possible that kInitTriggerId was set for a constructor call that
Expand All @@ -369,7 +369,7 @@
if (process.domain && process.domain._errorHandler)
caught = process.domain._errorHandler(er);

if (!caught)
if (!caught && !fromPromise)
caught = process.emit('uncaughtException', er);

// If someone handled it, then great. otherwise, die in C++ land
Expand Down
32 changes: 7 additions & 25 deletions lib/internal/process/promises.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,13 @@ const { safeToString } = process.binding('util');

const promiseRejectEvent = process._promiseRejectEvent;
const hasBeenNotifiedProperty = new WeakMap();
const promiseToGuidProperty = new WeakMap();
const pendingUnhandledRejections = [];
let lastPromiseId = 1;

exports.setup = setupPromises;

function getAsynchronousRejectionWarningObject(uid) {
return new Error('Promise rejection was handled ' +
`asynchronously (rejection id: ${uid})`);
}

function setupPromises(scheduleMicrotasks) {
let deprecationWarned = false;
const promiseInternals = {};

process._setupPromises(function(event, promise, reason) {
if (event === promiseRejectEvent.unhandled)
Expand All @@ -25,34 +19,23 @@ function setupPromises(scheduleMicrotasks) {
rejectionHandled(promise);
else
require('assert').fail(null, null, 'unexpected PromiseRejectEvent');
});
}, function getPromiseReason(data) {
return data[data.indexOf('[[PromiseValue]]') + 1];
}, promiseInternals);

function unhandledRejection(promise, reason) {
hasBeenNotifiedProperty.set(promise, false);
promiseToGuidProperty.set(promise, lastPromiseId++);
addPendingUnhandledRejection(promise, reason);
}

function rejectionHandled(promise) {
const hasBeenNotified = hasBeenNotifiedProperty.get(promise);
if (hasBeenNotified !== undefined) {
hasBeenNotifiedProperty.delete(promise);
const uid = promiseToGuidProperty.get(promise);
promiseToGuidProperty.delete(promise);
if (hasBeenNotified === true) {
let warning = null;
if (!process.listenerCount('rejectionHandled')) {
// Generate the warning object early to get a good stack trace.
warning = getAsynchronousRejectionWarningObject(uid);
}
promiseInternals.untrackPromise(promise);
process.nextTick(function() {
if (!process.emit('rejectionHandled', promise)) {
if (warning === null)
warning = getAsynchronousRejectionWarningObject(uid);
warning.name = 'PromiseRejectionHandledWarning';
warning.id = uid;
process.emitWarning(warning);
}
process.emit('rejectionHandled', promise);
});
}

Expand Down Expand Up @@ -90,9 +73,8 @@ function setupPromises(scheduleMicrotasks) {
const reason = pendingUnhandledRejections.shift();
if (hasBeenNotifiedProperty.get(promise) === false) {
hasBeenNotifiedProperty.set(promise, true);
const uid = promiseToGuidProperty.get(promise);
if (!process.emit('unhandledRejection', reason, promise)) {
emitWarning(uid, reason);
promiseInternals.trackPromise(promise);
} else {
hadListeners = true;
}
Expand Down
37 changes: 36 additions & 1 deletion node.gyp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@
'src/stream_wrap.cc',
'src/tcp_wrap.cc',
'src/timer_wrap.cc',
'src/track-promise.cc',
'src/tracing/agent.cc',
'src/tracing/node_trace_buffer.cc',
'src/tracing/node_trace_writer.cc',
Expand Down Expand Up @@ -270,6 +271,8 @@
'src/node_revert.h',
'src/node_i18n.h',
'src/pipe_wrap.h',
'src/track-promise.h',
'src/track-promise-inl.h',
'src/tty_wrap.h',
'src/tcp_wrap.h',
'src/udp_wrap.h',
Expand Down Expand Up @@ -654,7 +657,39 @@
'<(SHARED_INTERMEDIATE_DIR)', # for node_natives.h
],

'defines': [ 'NODE_WANT_INTERNALS=1' ],
'libraries': [
'<(OBJ_GEN_PATH)/node_javascript.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/node_debug_options.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/async-wrap.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/env.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/node.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/node_buffer.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/node_i18n.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/node_url.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/debug-agent.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/util.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/string_bytes.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/string_search.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/stream_base.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/node_constants.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/node_revert.<(OBJ_SUFFIX)',
'<(OBJ_TRACING_PATH)/agent.<(OBJ_SUFFIX)',
'<(OBJ_TRACING_PATH)/node_trace_buffer.<(OBJ_SUFFIX)',
'<(OBJ_TRACING_PATH)/node_trace_writer.<(OBJ_SUFFIX)',
'<(OBJ_TRACING_PATH)/trace_event.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/track-promise.<(OBJ_SUFFIX)',
],

'defines': [
# gtest's ASSERT macros conflict with our own.
'GTEST_DONT_DEFINE_ASSERT_EQ=1',
'GTEST_DONT_DEFINE_ASSERT_GE=1',
'GTEST_DONT_DEFINE_ASSERT_GT=1',
'GTEST_DONT_DEFINE_ASSERT_LE=1',
'GTEST_DONT_DEFINE_ASSERT_LT=1',
'GTEST_DONT_DEFINE_ASSERT_NE=1',
'NODE_WANT_INTERNALS=1',
],

'sources': [
'src/node_platform.cc',
Expand Down
3 changes: 2 additions & 1 deletion src/env-inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,8 @@ inline Environment* Environment::GetCurrent(

inline Environment::Environment(IsolateData* isolate_data,
v8::Local<v8::Context> context)
: isolate_(context->GetIsolate()),
: promise_tracker_(this),
isolate_(context->GetIsolate()),
isolate_data_(isolate_data),
async_hooks_(context->GetIsolate()),
timer_base_(uv_now(isolate_data->event_loop())),
Expand Down
11 changes: 9 additions & 2 deletions src/env.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
#endif
#include "handle_wrap.h"
#include "req-wrap.h"
#include "track-promise.h"
#include "tree.h"
#include "util.h"
#include "uv.h"
#include "v8.h"
Expand Down Expand Up @@ -236,6 +238,7 @@ class ModuleWrap;
V(preference_string, "preference") \
V(priority_string, "priority") \
V(produce_cached_data_string, "produceCachedData") \
V(promise_rejection_index_string, "_promiseRejectionIndex") \
V(raw_string, "raw") \
V(read_host_object_string, "_readHostObject") \
V(readable_string, "readable") \
Expand Down Expand Up @@ -294,6 +297,7 @@ class ModuleWrap;
V(zero_return_string, "ZERO_RETURN")

#define ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V) \
V(array_from, v8::Function) \
V(as_external, v8::External) \
V(async_hooks_destroy_function, v8::Function) \
V(async_hooks_init_function, v8::Function) \
Expand All @@ -312,8 +316,8 @@ class ModuleWrap;
V(performance_entry_callback, v8::Function) \
V(performance_entry_template, v8::Function) \
V(process_object, v8::Object) \
V(promise_reject_function, v8::Function) \
V(promise_wrap_template, v8::ObjectTemplate) \
V(promise_unhandled_rejection_function, v8::Function) \
V(promise_unhandled_rejection, v8::Function) \
V(push_values_to_array_function, v8::Function) \
V(randombytes_constructor_template, v8::ObjectTemplate) \
V(script_context_constructor_template, v8::FunctionTemplate) \
Expand Down Expand Up @@ -678,6 +682,9 @@ class Environment {
bool RemovePromiseHook(promise_hook_func fn, void* arg);
bool EmitNapiWarning();

PromiseTracker promise_tracker_;
int64_t promise_tracker_index_ = 0;

private:
inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
const char* errmsg);
Expand Down
Loading