1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

src: remove ClearFatalExceptionHandlers()

Browse Source 
At its call sites, `ClearFatalExceptionHandlers()` was used to
make the process crash as soon as possible once an exception occurred,
without giving JS land a chance to interfere.

`ClearFatalExceptionHandlers()` awkwardly removed the current domain
and any `uncaughtException` handlers, whereas a clearer way is to
execute the relevant reporting (and `exit()`) code directly.

PR-URL: https://github.com/nodejs/node/pull/17333
Refs: https://github.com/nodejs/node/pull/17159
Refs: https://github.com/nodejs/node/pull/17324
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This commit is contained in:
Anna Henningsen 2017-11-27 01:31:24 +01:00
parent 3c62f33d7b
commit a01267265a
No known key found for this signature in database
GPG Key ID: 9C63F3A6CD2AD8F9
4 changed files with 50 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
src/async_wrap.cc
View File

@ -140,7 +140,7 @@ RetainedObjectInfo* WrapperInfo(uint16_t class_id, Local<Value> wrapper) {
static void DestroyAsyncIdsCallback(Environment* env, void* data) {
Local<Function> fn = env->async_hooks_destroy_function();
TryCatch try_catch(env->isolate());
FatalTryCatch try_catch(env);
do {
std::vector<double> destroy_async_id_list;
@ -153,11 +153,8 @@ static void DestroyAsyncIdsCallback(Environment* env, void* data) {
MaybeLocal<Value> ret = fn->Call(
env->context(), Undefined(env->isolate()), 1, &async_id_value);
if (ret.IsEmpty()) {
ClearFatalExceptionHandlers(env);
FatalException(env->isolate(), try_catch);
UNREACHABLE();
}
if (ret.IsEmpty())
return;
}
} while (!env->destroy_async_id_list()->empty());
}
@ -171,14 +168,9 @@ void AsyncWrap::EmitPromiseResolve(Environment* env, double async_id) {
Local<Value> async_id_value = Number::New(env->isolate(), async_id);
Local<Function> fn = env->async_hooks_promise_resolve_function();
TryCatch try_catch(env->isolate());
MaybeLocal<Value> ar = fn->Call(
env->context(), Undefined(env->isolate()), 1, &async_id_value);
if (ar.IsEmpty()) {
ClearFatalExceptionHandlers(env);
FatalException(env->isolate(), try_catch);
UNREACHABLE();
}
FatalTryCatch try_catch(env);
fn->Call(env->context(), Undefined(env->isolate()), 1, &async_id_value)
.FromMaybe(Local<Value>());
}
@ -205,14 +197,9 @@ void AsyncWrap::EmitBefore(Environment* env, double async_id) {
Local<Value> async_id_value = Number::New(env->isolate(), async_id);
Local<Function> fn = env->async_hooks_before_function();
TryCatch try_catch(env->isolate());
MaybeLocal<Value> ar = fn->Call(
env->context(), Undefined(env->isolate()), 1, &async_id_value);
if (ar.IsEmpty()) {
ClearFatalExceptionHandlers(env);
FatalException(env->isolate(), try_catch);
UNREACHABLE();
}
FatalTryCatch try_catch(env);
fn->Call(env->context(), Undefined(env->isolate()), 1, &async_id_value)
.FromMaybe(Local<Value>());
}
@ -241,14 +228,9 @@ void AsyncWrap::EmitAfter(Environment* env, double async_id) {
// end of _fatalException().
Local<Value> async_id_value = Number::New(env->isolate(), async_id);
Local<Function> fn = env->async_hooks_after_function();
TryCatch try_catch(env->isolate());
MaybeLocal<Value> ar = fn->Call(
env->context(), Undefined(env->isolate()), 1, &async_id_value);
if (ar.IsEmpty()) {
ClearFatalExceptionHandlers(env);
FatalException(env->isolate(), try_catch);
UNREACHABLE();
}
FatalTryCatch try_catch(env);
fn->Call(env->context(), Undefined(env->isolate()), 1, &async_id_value)
.FromMaybe(Local<Value>());
}
class PromiseWrap : public AsyncWrap {
@ -748,14 +730,9 @@ void AsyncWrap::EmitAsyncInit(Environment* env,
object,
};
TryCatch try_catch(env->isolate());
MaybeLocal<Value> ret = init_fn->Call(
env->context(), object, arraysize(argv), argv);
if (ret.IsEmpty()) {
ClearFatalExceptionHandlers(env);
FatalException(env->isolate(), try_catch);
}
FatalTryCatch try_catch(env);
init_fn->Call(env->context(), object, arraysize(argv), argv)
.FromMaybe(Local<Value>());
}

37
src/node.cc
View File

@ -1471,6 +1471,8 @@ void AppendExceptionLine(Environment* env,
static void ReportException(Environment* env,
Local<Value> er,
Local<Message> message) {
CHECK(!er.IsEmpty());
CHECK(!message.IsEmpty());
HandleScope scope(env->isolate());
AppendExceptionLine(env, er, message, FATAL_ERROR);
@ -1540,6 +1542,10 @@ static void ReportException(Environment* env,
}
fflush(stderr);
#if HAVE_INSPECTOR
env->inspector_agent()->FatalException(er, message);
#endif
}
@ -2399,6 +2405,15 @@ NO_RETURN void FatalError(const char* location, const char* message) {
}
FatalTryCatch::~FatalTryCatch() {
if (HasCaught()) {
HandleScope scope(env_->isolate());
ReportException(env_, *this);
exit(7);
}
}
void FatalException(Isolate* isolate,
Local<Value> error,
Local<Message> message) {
@ -2441,9 +2456,6 @@ void FatalException(Isolate* isolate,
}
if (exit_code) {
#if HAVE_INSPECTOR
env->inspector_agent()->FatalException(error, message);
#endif
exit(exit_code);
}
}
@ -2463,25 +2475,6 @@ static void OnMessage(Local<Message> message, Local<Value> error) {
FatalException(Isolate::GetCurrent(), error, message);
}
void ClearFatalExceptionHandlers(Environment* env) {
Local<Object> process = env->process_object();
Local<Value> events =
process->Get(env->context(), env->events_string()).ToLocalChecked();
if (events->IsObject()) {
events.As<Object>()->Set(
env->context(),
OneByteString(env->isolate(), "uncaughtException"),
Undefined(env->isolate())).FromJust();
}
process->Set(
env->context(),
env->domain_string(),
Undefined(env->isolate())).FromJust();
}
// Call process.emitWarning(str), fmt is a snprintf() format string
void ProcessEmitWarning(Environment* env, const char* fmt, ...) {
char warning[1024];

16
src/node_internals.h
View File

@ -277,6 +277,17 @@ void AppendExceptionLine(Environment* env,
NO_RETURN void FatalError(const char* location, const char* message);
// Like a `TryCatch` but exits the process if an exception was caught.
class FatalTryCatch : public v8::TryCatch {
public:
explicit FatalTryCatch(Environment* env)
: TryCatch(env->isolate()), env_(env) {}
~FatalTryCatch();
private:
Environment* env_;
};
void ProcessEmitWarning(Environment* env, const char* fmt, ...);
void FillStatsArray(double* fields, const uv_stat_t* s);
@ -330,11 +341,6 @@ class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
uint32_t zero_fill_field_ = 1; // Boolean but exposed as uint32 to JS land.
};
// Clear any domain and/or uncaughtException handlers to force the error's
// propagation and shutdown the process. Use this to force the process to exit
// by clearing all callbacks that could handle the error.
void ClearFatalExceptionHandlers(Environment* env);
namespace Buffer {
v8::MaybeLocal<v8::Object> Copy(Environment* env, const char* data, size_t len);
v8::MaybeLocal<v8::Object> New(Environment* env, size_t size);

21
src/node_url.cc
View File

@ -2172,19 +2172,16 @@ const Local<Value> URL::ToObject(Environment* env) const {
};
SetArgs(env, argv, &context_);
TryCatch try_catch(isolate);
MaybeLocal<Value> ret;
{
FatalTryCatch try_catch(env);
// The SetURLConstructor method must have been called already to
// set the constructor function used below. SetURLConstructor is
// called automatically when the internal/url.js module is loaded
// during the internal/bootstrap_node.js processing.
MaybeLocal<Value> ret =
env->url_constructor_function()
->Call(env->context(), undef, 9, argv);
if (ret.IsEmpty()) {
ClearFatalExceptionHandlers(env);
FatalException(isolate, try_catch);
// The SetURLConstructor method must have been called already to
// set the constructor function used below. SetURLConstructor is
// called automatically when the internal/url.js module is loaded
// during the internal/bootstrap_node.js processing.
ret = env->url_constructor_function()
->Call(env->context(), undef, arraysize(argv), argv);
}
return ret.ToLocalChecked();