From 7206606c311be642268f9165b2074c9a945d2f47 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 4 Nov 2016 05:07:47 +0100 Subject: [PATCH 1/2] src: don't call into VM from AsyncWrap destructor It is not allowed anymore to call JS code when collecting weakly persistent handles, it hits the assertion below: # Fatal error in ../deps/v8/src/execution.cc, line 103 # Check failed: AllowJavascriptExecution::IsAllowed(isolate). Remove the call into the VM from the AsyncWrap destructor. This commit breaks the destroy hook but that cannot be helped. Fixes: https://github.com/nodejs/node/issues/8216 --- src/async-wrap-inl.h | 18 +----------------- .../test-async-wrap-throw-from-callback.js | 8 ++------ test/parallel/test-async-wrap-uid.js | 8 +------- 3 files changed, 4 insertions(+), 30 deletions(-) diff --git a/src/async-wrap-inl.h b/src/async-wrap-inl.h index 85e31b1ed09d27..f32e7995932c26 100644 --- a/src/async-wrap-inl.h +++ b/src/async-wrap-inl.h @@ -67,23 +67,7 @@ inline AsyncWrap::AsyncWrap(Environment* env, } -inline AsyncWrap::~AsyncWrap() { - if (!ran_init_callback()) - return; - - v8::Local fn = env()->async_hooks_destroy_function(); - if (!fn.IsEmpty()) { - v8::HandleScope scope(env()->isolate()); - v8::Local uid = v8::Number::New(env()->isolate(), get_uid()); - v8::TryCatch try_catch(env()->isolate()); - v8::MaybeLocal ret = - fn->Call(env()->context(), v8::Null(env()->isolate()), 1, &uid); - if (ret.IsEmpty()) { - ClearFatalExceptionHandlers(env()); - FatalException(env()->isolate(), try_catch); - } - } -} +inline AsyncWrap::~AsyncWrap() {} inline bool AsyncWrap::ran_init_callback() const { diff --git a/test/parallel/test-async-wrap-throw-from-callback.js b/test/parallel/test-async-wrap-throw-from-callback.js index 8b8316a65f9c2a..a02db4004a3ca2 100644 --- a/test/parallel/test-async-wrap-throw-from-callback.js +++ b/test/parallel/test-async-wrap-throw-from-callback.js @@ -11,7 +11,7 @@ const assert = require('assert'); const crypto = require('crypto'); const domain = require('domain'); const spawn = require('child_process').spawn; -const callbacks = [ 'init', 'pre', 'post', 'destroy' ]; +const callbacks = [ 'init', 'pre', 'post' ]; const toCall = process.argv[2]; var msgCalled = 0; var msgReceived = 0; @@ -28,13 +28,9 @@ function post() { if (toCall === 'post') throw new Error('post'); } -function destroy() { - if (toCall === 'destroy') - throw new Error('destroy'); -} if (typeof process.argv[2] === 'string') { - async_wrap.setupHooks({ init, pre, post, destroy }); + async_wrap.setupHooks({ init, pre, post }); async_wrap.enable(); process.on('uncaughtException', () => assert.ok(0, 'UNREACHABLE')); diff --git a/test/parallel/test-async-wrap-uid.js b/test/parallel/test-async-wrap-uid.js index 5bf3a8856e0e3f..3497c3b0768ddd 100644 --- a/test/parallel/test-async-wrap-uid.js +++ b/test/parallel/test-async-wrap-uid.js @@ -6,7 +6,7 @@ const assert = require('assert'); const async_wrap = process.binding('async_wrap'); const storage = new Map(); -async_wrap.setupHooks({ init, pre, post, destroy }); +async_wrap.setupHooks({ init, pre, post }); async_wrap.enable(); function init(uid) { @@ -14,7 +14,6 @@ function init(uid) { init: true, pre: false, post: false, - destroy: false }); } @@ -26,10 +25,6 @@ function post(uid) { storage.get(uid).post = true; } -function destroy(uid) { - storage.get(uid).destroy = true; -} - fs.access(__filename, function(err) { assert.ifError(err); }); @@ -51,7 +46,6 @@ process.once('exit', function() { init: true, pre: true, post: true, - destroy: true }); } }); From 1ffda6b33e8a86304a49abde5faf25e95caf181f Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 4 Nov 2016 17:24:43 +0100 Subject: [PATCH 2/2] squash! remove async_hooks_destroy_function --- src/async-wrap.cc | 6 ------ src/env.h | 1 - 2 files changed, 7 deletions(-) diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 60124e47ad8833..371f18c504eb0c 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -137,9 +137,6 @@ static void SetupHooks(const FunctionCallbackInfo& args) { Local post_v = fn_obj->Get( env->context(), FIXED_ONE_BYTE_STRING(env->isolate(), "post")).ToLocalChecked(); - Local destroy_v = fn_obj->Get( - env->context(), - FIXED_ONE_BYTE_STRING(env->isolate(), "destroy")).ToLocalChecked(); if (!init_v->IsFunction()) return env->ThrowTypeError("init callback must be a function"); @@ -150,8 +147,6 @@ static void SetupHooks(const FunctionCallbackInfo& args) { env->set_async_hooks_pre_function(pre_v.As()); if (post_v->IsFunction()) env->set_async_hooks_post_function(post_v.As()); - if (destroy_v->IsFunction()) - env->set_async_hooks_destroy_function(destroy_v.As()); } @@ -177,7 +172,6 @@ static void Initialize(Local target, env->set_async_hooks_init_function(Local()); env->set_async_hooks_pre_function(Local()); env->set_async_hooks_post_function(Local()); - env->set_async_hooks_destroy_function(Local()); } diff --git a/src/env.h b/src/env.h index fe49ac0c876a47..aec5a2d6f76fea 100644 --- a/src/env.h +++ b/src/env.h @@ -227,7 +227,6 @@ namespace node { #define ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V) \ V(as_external, v8::External) \ - V(async_hooks_destroy_function, v8::Function) \ V(async_hooks_init_function, v8::Function) \ V(async_hooks_post_function, v8::Function) \ V(async_hooks_pre_function, v8::Function) \