From eacd2e9c4cd27d60bd3d29f286e833070e6d7003 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 8 Aug 2017 19:56:02 +0200
Subject: [PATCH 1/8] src: refactor `#include` handling
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`node_internals.h` already includes the most common headers,
so double includes can be avoided in a lot of cases. Also don’t include
`node_internals.h` from `node.h` implicitly anymore, as that is mostly
unnecessary.

PR-URL: https://github.com/nodejs/node/pull/14697
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 src/async-wrap-inl.h            | 5 -----
 src/backtrace_posix.cc          | 2 +-
 src/env-inl.h                   | 1 +
 src/env.cc                      | 4 +---
 src/env.h                       | 4 ++++
 src/inspector_agent.cc          | 5 +----
 src/node.cc                     | 1 -
 src/node.h                      | 4 ----
 src/node_api.cc                 | 2 --
 src/node_buffer.h               | 3 +--
 src/node_constants.cc           | 4 +---
 src/node_contextify.cc          | 5 -----
 src/node_counters.h             | 2 +-
 src/node_dtrace.cc              | 5 +----
 src/node_dtrace.h               | 4 +---
 src/node_file.cc                | 4 ----
 src/node_i18n.h                 | 2 +-
 src/node_internals.h            | 2 ++
 src/node_javascript.h           | 3 +--
 src/node_main.cc                | 2 ++
 src/node_os.cc                  | 5 +----
 src/node_perf.cc                | 6 +-----
 src/node_url.cc                 | 6 ------
 src/node_util.cc                | 5 +----
 src/node_watchdog.cc            | 2 --
 src/string_search.h             | 2 +-
 src/tls_wrap.cc                 | 2 --
 src/util.cc                     | 1 -
 test/cctest/node_test_fixture.h | 1 +
 test/cctest/test_environment.cc | 4 +---
 30 files changed, 25 insertions(+), 73 deletions(-)

diff --git a/src/async-wrap-inl.h b/src/async-wrap-inl.h
index 4f8b7c3f8dd914..ed5a0c0d279048 100644
--- a/src/async-wrap-inl.h
+++ b/src/async-wrap-inl.h
@@ -27,12 +27,7 @@
 #include "async-wrap.h"
 #include "base-object.h"
 #include "base-object-inl.h"
-#include "env.h"
-#include "env-inl.h"
 #include "node_internals.h"
-#include "util.h"
-#include "util-inl.h"
-#include "v8.h"
 
 namespace node {
 
diff --git a/src/backtrace_posix.cc b/src/backtrace_posix.cc
index 8fd798757a544a..0c69d820e7212a 100644
--- a/src/backtrace_posix.cc
+++ b/src/backtrace_posix.cc
@@ -1,4 +1,4 @@
-#include "node.h"
+#include "node_internals.h"
 
 #if defined(__linux__)
 #include <features.h>
diff --git a/src/env-inl.h b/src/env-inl.h
index 404c06c6f453bd..ddfb5fd93ffcf3 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -31,6 +31,7 @@
 #include "util-inl.h"
 #include "uv.h"
 #include "v8.h"
+#include "node_perf_common.h"
 
 #include <stddef.h>
 #include <stdint.h>
diff --git a/src/env.cc b/src/env.cc
index 076198cd626a81..ba5caa1ec3acca 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -1,7 +1,5 @@
-#include "env.h"
-#include "env-inl.h"
+#include "node_internals.h"
 #include "async-wrap.h"
-#include "v8.h"
 #include "v8-profiler.h"
 
 #if defined(_MSC_VER)
diff --git a/src/env.h b/src/env.h
index 13b1f4cb137bce..4af87192df8128 100644
--- a/src/env.h
+++ b/src/env.h
@@ -48,6 +48,10 @@ struct nghttp2_rcbuf;
 
 namespace node {
 
+namespace performance {
+struct performance_state;
+}
+
 // Pick an index that's hopefully out of the way when we're embedded inside
 // another application. Performance-wise or memory-wise it doesn't matter:
 // Context::SetAlignedPointerInEmbedderData() is backed by a FixedArray,
diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc
index 828006ecf2fbb4..9f33b3d31e69e6 100644
--- a/src/inspector_agent.cc
+++ b/src/inspector_agent.cc
@@ -1,12 +1,9 @@
 #include "inspector_agent.h"
 
 #include "inspector_io.h"
-#include "env.h"
-#include "env-inl.h"
-#include "node.h"
+#include "node_internals.h"
 #include "v8-inspector.h"
 #include "v8-platform.h"
-#include "util.h"
 #include "zlib.h"
 
 #include "libplatform/libplatform.h"
diff --git a/src/node.cc b/src/node.cc
index 664ae22a9ae258..30cbb11efa9e25 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -19,7 +19,6 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-#include "node.h"
 #include "node_buffer.h"
 #include "node_constants.h"
 #include "node_javascript.h"
diff --git a/src/node.h b/src/node.h
index fbd17d2ca09c7f..5fe7bde4289ae4 100644
--- a/src/node.h
+++ b/src/node.h
@@ -169,10 +169,6 @@ NODE_EXTERN v8::Local<v8::Value> MakeCallback(
 
 }  // namespace node
 
-#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
-#include "node_internals.h"
-#endif
-
 #include <assert.h>
 #include <stdint.h>
 
diff --git a/src/node_api.cc b/src/node_api.cc
index 7bb97c8076fac3..8c55e64b7cda05 100644
--- a/src/node_api.cc
+++ b/src/node_api.cc
@@ -14,10 +14,8 @@
 #include <algorithm>
 #include <cmath>
 #include <vector>
-#include "uv.h"
 #include "node_api.h"
 #include "node_internals.h"
-#include "util.h"
 
 #define NAPI_VERSION  1
 
diff --git a/src/node_buffer.h b/src/node_buffer.h
index acf9b23c3b3256..d680eb90eb1649 100644
--- a/src/node_buffer.h
+++ b/src/node_buffer.h
@@ -22,8 +22,7 @@
 #ifndef SRC_NODE_BUFFER_H_
 #define SRC_NODE_BUFFER_H_
 
-#include "node.h"
-#include "v8.h"
+#include "node_internals.h"
 
 namespace node {
 
diff --git a/src/node_constants.cc b/src/node_constants.cc
index b8b97b21f9ea39..7fd303dd32fed0 100644
--- a/src/node_constants.cc
+++ b/src/node_constants.cc
@@ -20,10 +20,8 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 #include "node_constants.h"
-#include "env.h"
-#include "env-inl.h"
+#include "node_internals.h"
 
-#include "uv.h"
 #include "zlib.h"
 
 #include <errno.h>
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index b04bd6253e9386..c73db420f18b4e 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -19,15 +19,10 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-#include "node.h"
 #include "node_internals.h"
 #include "node_watchdog.h"
 #include "base-object.h"
 #include "base-object-inl.h"
-#include "env.h"
-#include "env-inl.h"
-#include "util.h"
-#include "util-inl.h"
 #include "v8-debug.h"
 
 namespace node {
diff --git a/src/node_counters.h b/src/node_counters.h
index 5d866aedb57677..c8a1a88f0b25e0 100644
--- a/src/node_counters.h
+++ b/src/node_counters.h
@@ -24,7 +24,7 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
-#include "node.h"
+#include "node_internals.h"
 
 #ifdef HAVE_PERFCTR
 #include "node_win32_perfctr_provider.h"
diff --git a/src/node_dtrace.cc b/src/node_dtrace.cc
index 94d06a7404ed33..4ff2f22d8e835a 100644
--- a/src/node_dtrace.cc
+++ b/src/node_dtrace.cc
@@ -43,10 +43,7 @@
 #define NODE_GC_DONE(arg0, arg1, arg2)
 #endif
 
-#include "env.h"
-#include "env-inl.h"
-
-#include "util.h"
+#include "node_internals.h"
 
 #include <string.h>
 
diff --git a/src/node_dtrace.h b/src/node_dtrace.h
index c22bf4e7fc0f12..d16e23ddb384b7 100644
--- a/src/node_dtrace.h
+++ b/src/node_dtrace.h
@@ -24,9 +24,7 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
-#include "node.h"
-#include "v8.h"
-#include "env.h"
+#include "node_internals.h"
 
 extern "C" {
 /*
diff --git a/src/node_file.cc b/src/node_file.cc
index cf7f1df5eb98bc..b9b3d34f346e56 100644
--- a/src/node_file.cc
+++ b/src/node_file.cc
@@ -19,17 +19,13 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-#include "node.h"
 #include "node_buffer.h"
 #include "node_internals.h"
 #include "node_stat_watcher.h"
 
-#include "env.h"
-#include "env-inl.h"
 #include "req-wrap.h"
 #include "req-wrap-inl.h"
 #include "string_bytes.h"
-#include "util.h"
 
 #include <fcntl.h>
 #include <sys/types.h>
diff --git a/src/node_i18n.h b/src/node_i18n.h
index adf9feb414df5c..70a0c79f76cf30 100644
--- a/src/node_i18n.h
+++ b/src/node_i18n.h
@@ -24,7 +24,7 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
-#include "node.h"
+#include "node_internals.h"
 #include <string>
 
 #if defined(NODE_HAVE_I18N_SUPPORT)
diff --git a/src/node_internals.h b/src/node_internals.h
index 6faf2750d4d039..9b6fae9d6a9cdd 100644
--- a/src/node_internals.h
+++ b/src/node_internals.h
@@ -27,6 +27,8 @@
 #include "node.h"
 #include "util.h"
 #include "util-inl.h"
+#include "env.h"
+#include "env-inl.h"
 #include "uv.h"
 #include "v8.h"
 #include "tracing/trace_event.h"
diff --git a/src/node_javascript.h b/src/node_javascript.h
index 3e8528fd211fb6..664778091ff669 100644
--- a/src/node_javascript.h
+++ b/src/node_javascript.h
@@ -24,8 +24,7 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
-#include "v8.h"
-#include "env.h"
+#include "node_internals.h"
 
 namespace node {
 
diff --git a/src/node_main.cc b/src/node_main.cc
index 7d6d9b1a01bbd4..2a511b92996e86 100644
--- a/src/node_main.cc
+++ b/src/node_main.cc
@@ -20,8 +20,10 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 #include "node.h"
+#include <stdio.h>
 
 #ifdef _WIN32
+#include <windows.h>
 #include <VersionHelpers.h>
 #include <WinError.h>
 
diff --git a/src/node_os.cc b/src/node_os.cc
index c71ca401ed6609..f09cd6fa5a03ff 100644
--- a/src/node_os.cc
+++ b/src/node_os.cc
@@ -19,10 +19,7 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-#include "node.h"
-#include "v8.h"
-#include "env.h"
-#include "env-inl.h"
+#include "node_internals.h"
 #include "string_bytes.h"
 
 #include <errno.h>
diff --git a/src/node_perf.cc b/src/node_perf.cc
index 48917d5d4ea971..098cf35220a400 100644
--- a/src/node_perf.cc
+++ b/src/node_perf.cc
@@ -1,9 +1,5 @@
-#include "node.h"
-#include "v8.h"
-#include "env.h"
-#include "env-inl.h"
+#include "node_internals.h"
 #include "node_perf.h"
-#include "uv.h"
 
 #include <vector>
 
diff --git a/src/node_url.cc b/src/node_url.cc
index f8adc7d7af5509..20a869ad7afc63 100644
--- a/src/node_url.cc
+++ b/src/node_url.cc
@@ -1,11 +1,5 @@
 #include "node_url.h"
-#include "node.h"
 #include "node_internals.h"
-#include "env.h"
-#include "env-inl.h"
-#include "util.h"
-#include "util-inl.h"
-#include "v8.h"
 #include "base-object.h"
 #include "base-object-inl.h"
 #include "node_i18n.h"
diff --git a/src/node_util.cc b/src/node_util.cc
index bbbea9ea2254dd..ab1f3c9f91257d 100644
--- a/src/node_util.cc
+++ b/src/node_util.cc
@@ -1,8 +1,5 @@
-#include "node.h"
+#include "node_internals.h"
 #include "node_watchdog.h"
-#include "v8.h"
-#include "env.h"
-#include "env-inl.h"
 
 namespace node {
 namespace util {
diff --git a/src/node_watchdog.cc b/src/node_watchdog.cc
index f4020e56f7fb61..af4e9f6fcfe3c2 100644
--- a/src/node_watchdog.cc
+++ b/src/node_watchdog.cc
@@ -21,8 +21,6 @@
 
 #include "node_watchdog.h"
 #include "node_internals.h"
-#include "util.h"
-#include "util-inl.h"
 #include <algorithm>
 
 namespace node {
diff --git a/src/string_search.h b/src/string_search.h
index dfdb8e9a160460..73e90f5873f767 100644
--- a/src/string_search.h
+++ b/src/string_search.h
@@ -7,7 +7,7 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
-#include "node.h"
+#include "node_internals.h"
 #include <string.h>
 
 namespace node {
diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
index b5829cf5b82f6c..dce05fabd62160 100644
--- a/src/tls_wrap.cc
+++ b/src/tls_wrap.cc
@@ -31,8 +31,6 @@
 #include "node_internals.h"
 #include "stream_base.h"
 #include "stream_base-inl.h"
-#include "util.h"
-#include "util-inl.h"
 
 namespace node {
 
diff --git a/src/util.cc b/src/util.cc
index 4a89b3a42f24b0..ef93d16968e465 100644
--- a/src/util.cc
+++ b/src/util.cc
@@ -19,7 +19,6 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-#include "util.h"
 #include "string_bytes.h"
 #include "node_buffer.h"
 #include "node_internals.h"
diff --git a/test/cctest/node_test_fixture.h b/test/cctest/node_test_fixture.h
index 79027d25ad8c8d..263f7b96f9daec 100644
--- a/test/cctest/node_test_fixture.h
+++ b/test/cctest/node_test_fixture.h
@@ -4,6 +4,7 @@
 #include <stdlib.h>
 #include "gtest/gtest.h"
 #include "node.h"
+#include "node_platform.h"
 #include "env.h"
 #include "v8.h"
 #include "libplatform/libplatform.h"
diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc
index 4651e865a99e7d..8beacfa95ece7e 100644
--- a/test/cctest/test_environment.cc
+++ b/test/cctest/test_environment.cc
@@ -1,6 +1,4 @@
-#include "node.h"
-#include "env.h"
-#include "v8.h"
+#include "node_internals.h"
 #include "libplatform/libplatform.h"
 
 #include <string>

From 95976d807c793a187055a4c3b1b37a1c7a1f5d55 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 8 Aug 2017 20:01:18 +0200
Subject: [PATCH 2/8] src: make in_makecallback() getter const
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

PR-URL: https://github.com/nodejs/node/pull/14697
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 src/env-inl.h | 2 +-
 src/env.h     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/env-inl.h b/src/env-inl.h
index ddfb5fd93ffcf3..93518fe5aefaa2 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -223,7 +223,7 @@ inline Environment::AsyncCallbackScope::~AsyncCallbackScope() {
   env_->makecallback_cntr_--;
 }
 
-inline bool Environment::AsyncCallbackScope::in_makecallback() {
+inline bool Environment::AsyncCallbackScope::in_makecallback() const {
   return env_->makecallback_cntr_ > 1;
 }
 
diff --git a/src/env.h b/src/env.h
index 4af87192df8128..dcaeefd226096c 100644
--- a/src/env.h
+++ b/src/env.h
@@ -463,7 +463,7 @@ class Environment {
     AsyncCallbackScope() = delete;
     explicit AsyncCallbackScope(Environment* env);
     ~AsyncCallbackScope();
-    inline bool in_makecallback();
+    inline bool in_makecallback() const;
 
    private:
     Environment* env_;

From f3ef344ac927a9ad67f35c8d1f0ec6f61ec670b4 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 8 Aug 2017 20:02:55 +0200
Subject: [PATCH 3/8] src: refactor async callback handling
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Merge the two almost-but-not-quite identical `MakeCallback()`
  implementations
- Provide a public `CallbackScope` class for embedders in order
  to enable `MakeCallback()`-like behaviour without tying that
  to calling a JS function

PR-URL: https://github.com/nodejs/node/pull/14697
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 src/async-wrap.cc                             |  69 +------
 src/node.cc                                   | 169 ++++++++++++------
 src/node.h                                    |  40 +++++
 src/node_internals.h                          |   8 +
 test/addons/callback-scope/binding.cc         |  39 ++++
 test/addons/callback-scope/binding.gyp        |   9 +
 .../addons/callback-scope/test-async-hooks.js |  22 +++
 test/addons/callback-scope/test.js            |  17 ++
 8 files changed, 256 insertions(+), 117 deletions(-)
 create mode 100644 test/addons/callback-scope/binding.cc
 create mode 100644 test/addons/callback-scope/binding.gyp
 create mode 100644 test/addons/callback-scope/test-async-hooks.js
 create mode 100644 test/addons/callback-scope/test.js

diff --git a/src/async-wrap.cc b/src/async-wrap.cc
index 391a684759cd03..fe53d279947a96 100644
--- a/src/async-wrap.cc
+++ b/src/async-wrap.cc
@@ -657,73 +657,8 @@ void AsyncWrap::EmitAsyncInit(Environment* env,
 MaybeLocal<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
                                           int argc,
                                           Local<Value>* argv) {
-  CHECK(env()->context() == env()->isolate()->GetCurrentContext());
-
-  Environment::AsyncCallbackScope callback_scope(env());
-
-  Environment::AsyncHooks::ExecScope exec_scope(env(),
-                                                get_id(),
-                                                get_trigger_id());
-
-  // Return v8::Undefined() because returning an empty handle will cause
-  // ToLocalChecked() to abort.
-  if (env()->using_domains() && DomainEnter(env(), object())) {
-    return Undefined(env()->isolate());
-  }
-
-  // No need to check a return value because the application will exit if an
-  // exception occurs.
-  AsyncWrap::EmitBefore(env(), get_id());
-
-  MaybeLocal<Value> ret = cb->Call(env()->context(), object(), argc, argv);
-
-  if (ret.IsEmpty()) {
-    return ret;
-  }
-
-  AsyncWrap::EmitAfter(env(), get_id());
-
-  // Return v8::Undefined() because returning an empty handle will cause
-  // ToLocalChecked() to abort.
-  if (env()->using_domains() && DomainExit(env(), object())) {
-    return Undefined(env()->isolate());
-  }
-
-  exec_scope.Dispose();
-
-  if (callback_scope.in_makecallback()) {
-    return ret;
-  }
-
-  Environment::TickInfo* tick_info = env()->tick_info();
-
-  if (tick_info->length() == 0) {
-    env()->isolate()->RunMicrotasks();
-  }
-
-  // Make sure the stack unwound properly. If there are nested MakeCallback's
-  // then it should return early and not reach this code.
-  CHECK_EQ(env()->current_async_id(), 0);
-  CHECK_EQ(env()->trigger_id(), 0);
-
-  Local<Object> process = env()->process_object();
-
-  if (tick_info->length() == 0) {
-    tick_info->set_index(0);
-    return ret;
-  }
-
-  MaybeLocal<Value> rcheck =
-      env()->tick_callback_function()->Call(env()->context(),
-                                            process,
-                                            0,
-                                            nullptr);
-
-  // Make sure the stack unwound properly.
-  CHECK_EQ(env()->current_async_id(), 0);
-  CHECK_EQ(env()->trigger_id(), 0);
-
-  return rcheck.IsEmpty() ? MaybeLocal<Value>() : ret;
+  async_context context { get_id(), get_trigger_id() };
+  return InternalMakeCallback(env(), object(), cb, argc, argv, context);
 }
 
 
diff --git a/src/node.cc b/src/node.cc
index 30cbb11efa9e25..cdbb7edb9eb0d7 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -161,6 +161,7 @@ using v8::SealHandleScope;
 using v8::String;
 using v8::TryCatch;
 using v8::Uint32Array;
+using v8::Undefined;
 using v8::V8;
 using v8::Value;
 
@@ -1311,85 +1312,153 @@ void AddPromiseHook(v8::Isolate* isolate, promise_hook_func fn, void* arg) {
   env->AddPromiseHook(fn, arg);
 }
 
+class InternalCallbackScope {
+ public:
+  InternalCallbackScope(Environment* env,
+                        Local<Object> object,
+                        const async_context& asyncContext);
+  ~InternalCallbackScope();
+  void Close();
+
+  inline bool Failed() const { return failed_; }
+  inline void MarkAsFailed() { failed_ = true; }
+  inline bool IsInnerMakeCallback() const {
+    return callback_scope_.in_makecallback();
+  }
+
+ private:
+  Environment* env_;
+  async_context async_context_;
+  v8::Local<v8::Object> object_;
+  Environment::AsyncCallbackScope callback_scope_;
+  bool failed_ = false;
+  bool pushed_ids_ = false;
+  bool closed_ = false;
+};
 
-MaybeLocal<Value> MakeCallback(Environment* env,
-                               Local<Value> recv,
-                               const Local<Function> callback,
-                               int argc,
-                               Local<Value> argv[],
-                               async_context asyncContext) {
-  // If you hit this assertion, you forgot to enter the v8::Context first.
-  CHECK_EQ(env->context(), env->isolate()->GetCurrentContext());
+CallbackScope::CallbackScope(Isolate* isolate,
+                             Local<Object> object,
+                             async_context asyncContext)
+  : private_(new InternalCallbackScope(Environment::GetCurrent(isolate),
+                                       object,
+                                       asyncContext)),
+    try_catch_(isolate) {
+  try_catch_.SetVerbose(true);
+}
 
-  Local<Object> object;
+CallbackScope::~CallbackScope() {
+  if (try_catch_.HasCaught())
+    private_->MarkAsFailed();
+  delete private_;
+}
 
-  Environment::AsyncCallbackScope callback_scope(env);
-  bool disposed_domain = false;
+InternalCallbackScope::InternalCallbackScope(Environment* env,
+                                             Local<Object> object,
+                                             const async_context& asyncContext)
+  : env_(env),
+    async_context_(asyncContext),
+    object_(object),
+    callback_scope_(env) {
+  CHECK(!object.IsEmpty());
 
-  if (recv->IsObject()) {
-    object = recv.As<Object>();
-  }
+  // If you hit this assertion, you forgot to enter the v8::Context first.
+  CHECK_EQ(env->context(), env->isolate()->GetCurrentContext());
 
   if (env->using_domains()) {
-    CHECK(recv->IsObject());
-    disposed_domain = DomainEnter(env, object);
-    if (disposed_domain) return Undefined(env->isolate());
+    failed_ = DomainEnter(env, object_);
+    if (failed_)
+      return;
   }
 
-  MaybeLocal<Value> ret;
+  if (asyncContext.async_id != 0) {
+    // No need to check a return value because the application will exit if
+    // an exception occurs.
+    AsyncWrap::EmitBefore(env, asyncContext.async_id);
+  }
 
-  {
-    AsyncHooks::ExecScope exec_scope(env, asyncContext.async_id,
-                                     asyncContext.trigger_async_id);
+  env->async_hooks()->push_ids(async_context_.async_id,
+                               async_context_.trigger_async_id);
+  pushed_ids_ = true;
+}
 
-    if (asyncContext.async_id != 0) {
-      // No need to check a return value because the application will exit if
-      // an exception occurs.
-      AsyncWrap::EmitBefore(env, asyncContext.async_id);
-    }
+InternalCallbackScope::~InternalCallbackScope() {
+  Close();
+}
 
-    ret = callback->Call(env->context(), recv, argc, argv);
+void InternalCallbackScope::Close() {
+  if (closed_) return;
+  closed_ = true;
 
-    if (ret.IsEmpty()) {
-      // NOTE: For backwards compatibility with public API we return Undefined()
-      // if the top level call threw.
-      return callback_scope.in_makecallback() ?
-          ret : Undefined(env->isolate());
-    }
+  if (pushed_ids_)
+    env_->async_hooks()->pop_ids(async_context_.async_id);
 
-    if (asyncContext.async_id != 0) {
-      AsyncWrap::EmitAfter(env, asyncContext.async_id);
-    }
+  if (failed_) return;
+
+  if (async_context_.async_id != 0) {
+    AsyncWrap::EmitAfter(env_, async_context_.async_id);
   }
 
-  if (env->using_domains()) {
-    disposed_domain = DomainExit(env, object);
-    if (disposed_domain) return Undefined(env->isolate());
+  if (env_->using_domains()) {
+    failed_ = DomainExit(env_, object_);
+    if (failed_) return;
   }
 
-  if (callback_scope.in_makecallback()) {
-    return ret;
+  if (IsInnerMakeCallback()) {
+    return;
   }
 
-  Environment::TickInfo* tick_info = env->tick_info();
+  Environment::TickInfo* tick_info = env_->tick_info();
 
   if (tick_info->length() == 0) {
-    env->isolate()->RunMicrotasks();
+    env_->isolate()->RunMicrotasks();
   }
 
   // Make sure the stack unwound properly. If there are nested MakeCallback's
   // then it should return early and not reach this code.
-  CHECK_EQ(env->current_async_id(), asyncContext.async_id);
-  CHECK_EQ(env->trigger_id(), asyncContext.trigger_async_id);
+  CHECK_EQ(env_->current_async_id(), 0);
+  CHECK_EQ(env_->trigger_id(), 0);
 
-  Local<Object> process = env->process_object();
+  Local<Object> process = env_->process_object();
 
   if (tick_info->length() == 0) {
     tick_info->set_index(0);
-    return ret;
+    return;
+  }
+
+  CHECK_EQ(env_->current_async_id(), 0);
+  CHECK_EQ(env_->trigger_id(), 0);
+
+  if (env_->tick_callback_function()->Call(process, 0, nullptr).IsEmpty()) {
+    failed_ = true;
+  }
+}
+
+MaybeLocal<Value> InternalMakeCallback(Environment* env,
+                                       Local<Object> recv,
+                                       const Local<Function> callback,
+                                       int argc,
+                                       Local<Value> argv[],
+                                       async_context asyncContext) {
+  InternalCallbackScope scope(env, recv, asyncContext);
+  if (scope.Failed()) {
+    return Undefined(env->isolate());
+  }
+
+  MaybeLocal<Value> ret;
+
+  {
+    ret = callback->Call(env->context(), recv, argc, argv);
+
+    if (ret.IsEmpty()) {
+      // NOTE: For backwards compatibility with public API we return Undefined()
+      // if the top level call threw.
+      scope.MarkAsFailed();
+      return scope.IsInnerMakeCallback() ? ret : Undefined(env->isolate());
+    }
   }
 
-  if (env->tick_callback_function()->Call(process, 0, nullptr).IsEmpty()) {
+  scope.Close();
+  if (scope.Failed()) {
     return Undefined(env->isolate());
   }
 
@@ -1442,8 +1511,8 @@ MaybeLocal<Value> MakeCallback(Isolate* isolate,
   // the two contexts need not be the same.
   Environment* env = Environment::GetCurrent(callback->CreationContext());
   Context::Scope context_scope(env->context());
-  return MakeCallback(env, recv.As<Value>(), callback, argc, argv,
-                      asyncContext);
+  return InternalMakeCallback(env, recv, callback,
+                              argc, argv, asyncContext);
 }
 
 
diff --git a/src/node.h b/src/node.h
index 5fe7bde4289ae4..6558a9ee6da96e 100644
--- a/src/node.h
+++ b/src/node.h
@@ -570,6 +570,36 @@ NODE_EXTERN async_context EmitAsyncInit(v8::Isolate* isolate,
 NODE_EXTERN void EmitAsyncDestroy(v8::Isolate* isolate,
                                   async_context asyncContext);
 
+class InternalCallbackScope;
+
+/* This class works like `MakeCallback()` in that it sets up a specific
+ * asyncContext as the current one and informs the async_hooks and domains
+ * modules that this context is currently active.
+ *
+ * `MakeCallback()` is a wrapper around this class as well as
+ * `Function::Call()`. Either one of these mechanisms needs to be used for
+ * top-level calls into JavaScript (i.e. without any existing JS stack).
+ *
+ * This object should be stack-allocated to ensure that it is contained in a
+ * valid HandleScope.
+ */
+class NODE_EXTERN CallbackScope {
+ public:
+  CallbackScope(v8::Isolate* isolate,
+                v8::Local<v8::Object> resource,
+                async_context asyncContext);
+  ~CallbackScope();
+
+ private:
+  InternalCallbackScope* private_;
+  v8::TryCatch try_catch_;
+
+  void operator=(const CallbackScope&) = delete;
+  void operator=(CallbackScope&&) = delete;
+  CallbackScope(const CallbackScope&) = delete;
+  CallbackScope(CallbackScope&&) = delete;
+};
+
 /* An API specific to emit before/after callbacks is unnecessary because
  * MakeCallback will automatically call them for you.
  *
@@ -659,6 +689,16 @@ class AsyncResource {
     async_id get_trigger_async_id() const {
       return async_context_.trigger_async_id;
     }
+
+  protected:
+    class CallbackScope : public node::CallbackScope {
+     public:
+      explicit CallbackScope(AsyncResource* res)
+        : node::CallbackScope(res->isolate_,
+                              res->resource_.Get(res->isolate_),
+                              res->async_context_) {}
+    };
+
   private:
     v8::Isolate* isolate_;
     v8::Persistent<v8::Object> resource_;
diff --git a/src/node_internals.h b/src/node_internals.h
index 9b6fae9d6a9cdd..9371d442ad0ea5 100644
--- a/src/node_internals.h
+++ b/src/node_internals.h
@@ -286,6 +286,14 @@ static v8::MaybeLocal<v8::Object> New(Environment* env,
 }
 }  // namespace Buffer
 
+v8::MaybeLocal<v8::Value> InternalMakeCallback(
+    Environment* env,
+    v8::Local<v8::Object> recv,
+    const v8::Local<v8::Function> callback,
+    int argc,
+    v8::Local<v8::Value> argv[],
+    async_context asyncContext);
+
 }  // namespace node
 
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
diff --git a/test/addons/callback-scope/binding.cc b/test/addons/callback-scope/binding.cc
new file mode 100644
index 00000000000000..31317e5dae236f
--- /dev/null
+++ b/test/addons/callback-scope/binding.cc
@@ -0,0 +1,39 @@
+#include "node.h"
+#include "v8.h"
+
+#include <assert.h>
+#include <vector>
+
+namespace {
+
+void RunInCallbackScope(const v8::FunctionCallbackInfo<v8::Value>& args) {
+  v8::Isolate* isolate = args.GetIsolate();
+
+  assert(args.Length() == 4);
+  assert(args[0]->IsObject());
+  assert(args[1]->IsNumber());
+  assert(args[2]->IsNumber());
+  assert(args[3]->IsFunction());
+
+  node::async_context asyncContext = {
+    args[1].As<v8::Number>()->Value(),
+    args[2].As<v8::Number>()->Value()
+  };
+
+  node::CallbackScope scope(isolate, args[0].As<v8::Object>(), asyncContext);
+  v8::Local<v8::Function> fn = args[3].As<v8::Function>();
+
+  v8::MaybeLocal<v8::Value> ret =
+      fn->Call(isolate->GetCurrentContext(), args[0], 0, nullptr);
+
+  if (!ret.IsEmpty())
+    args.GetReturnValue().Set(ret.ToLocalChecked());
+}
+
+void Initialize(v8::Local<v8::Object> exports) {
+  NODE_SET_METHOD(exports, "runInCallbackScope", RunInCallbackScope);
+}
+
+}  // namespace
+
+NODE_MODULE(binding, Initialize)
diff --git a/test/addons/callback-scope/binding.gyp b/test/addons/callback-scope/binding.gyp
new file mode 100644
index 00000000000000..7ede63d94a0d77
--- /dev/null
+++ b/test/addons/callback-scope/binding.gyp
@@ -0,0 +1,9 @@
+{
+  'targets': [
+    {
+      'target_name': 'binding',
+      'defines': [ 'V8_DEPRECATION_WARNINGS=1' ],
+      'sources': [ 'binding.cc' ]
+    }
+  ]
+}
diff --git a/test/addons/callback-scope/test-async-hooks.js b/test/addons/callback-scope/test-async-hooks.js
new file mode 100644
index 00000000000000..94b53efc53d78e
--- /dev/null
+++ b/test/addons/callback-scope/test-async-hooks.js
@@ -0,0 +1,22 @@
+'use strict';
+
+const common = require('../../common');
+const assert = require('assert');
+const async_hooks = require('async_hooks');
+const { runInCallbackScope } = require(`./build/${common.buildType}/binding`);
+
+let insideHook = false;
+async_hooks.createHook({
+  before: common.mustCall((id) => {
+    assert.strictEqual(id, 1000);
+    insideHook = true;
+  }),
+  after: common.mustCall((id) => {
+    assert.strictEqual(id, 1000);
+    insideHook = false;
+  })
+}).enable();
+
+runInCallbackScope({}, 1000, 1000, () => {
+  assert(insideHook);
+});
diff --git a/test/addons/callback-scope/test.js b/test/addons/callback-scope/test.js
new file mode 100644
index 00000000000000..2f2efe5f47b98a
--- /dev/null
+++ b/test/addons/callback-scope/test.js
@@ -0,0 +1,17 @@
+'use strict';
+
+const common = require('../../common');
+const assert = require('assert');
+const { runInCallbackScope } = require(`./build/${common.buildType}/binding`);
+
+assert.strictEqual(runInCallbackScope({}, 0, 0, () => 42), 42);
+
+{
+  process.once('uncaughtException', common.mustCall((err) => {
+    assert.strictEqual(err.message, 'foo');
+  }));
+
+  runInCallbackScope({}, 0, 0, () => {
+    throw new Error('foo');
+  });
+}

From 4262fadbf7c3f2ed0d83690c013862df02f0c3a0 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 8 Aug 2017 20:08:08 +0200
Subject: [PATCH 4/8] src: remove virtually unused ExecScope
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

PR-URL: https://github.com/nodejs/node/pull/14697
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 src/env-inl.h | 20 --------------------
 src/env.h     | 19 -------------------
 src/node.cc   |  3 ++-
 3 files changed, 2 insertions(+), 40 deletions(-)

diff --git a/src/env-inl.h b/src/env-inl.h
index 93518fe5aefaa2..eddec5ca54fb48 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -194,26 +194,6 @@ inline Environment::AsyncHooks::InitScope::~InitScope() {
   env_->async_hooks()->pop_ids(uid_fields_ref_[AsyncHooks::kCurrentAsyncId]);
 }
 
-inline Environment::AsyncHooks::ExecScope::ExecScope(
-    Environment* env, double async_id, double trigger_id)
-        : env_(env),
-          async_id_(async_id),
-          disposed_(false) {
-  CHECK_GE(async_id, -1);
-  CHECK_GE(trigger_id, -1);
-  env->async_hooks()->push_ids(async_id, trigger_id);
-}
-
-inline Environment::AsyncHooks::ExecScope::~ExecScope() {
-  if (disposed_) return;
-  Dispose();
-}
-
-inline void Environment::AsyncHooks::ExecScope::Dispose() {
-  disposed_ = true;
-  env_->async_hooks()->pop_ids(async_id_);
-}
-
 inline Environment::AsyncCallbackScope::AsyncCallbackScope(Environment* env)
     : env_(env) {
   env_->makecallback_cntr_++;
diff --git a/src/env.h b/src/env.h
index dcaeefd226096c..315d4fdc1852a4 100644
--- a/src/env.h
+++ b/src/env.h
@@ -421,25 +421,6 @@ class Environment {
       DISALLOW_COPY_AND_ASSIGN(InitScope);
     };
 
-    // Used to manage the stack of async and trigger ids as calls are made into
-    // JS. Mainly used in MakeCallback().
-    class ExecScope {
-     public:
-      ExecScope() = delete;
-      explicit ExecScope(Environment* env, double async_id, double trigger_id);
-      ~ExecScope();
-      void Dispose();
-
-     private:
-      Environment* env_;
-      double async_id_;
-      // Manually track if the destructor has run so it isn't accidentally run
-      // twice on RAII cleanup.
-      bool disposed_;
-
-      DISALLOW_COPY_AND_ASSIGN(ExecScope);
-    };
-
    private:
     friend class Environment;  // So we can call the constructor.
     inline explicit AsyncHooks(v8::Isolate* isolate);
diff --git a/src/node.cc b/src/node.cc
index cdbb7edb9eb0d7..3cfaf8d12944c8 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -4692,8 +4692,9 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
 
   {
     Environment::AsyncCallbackScope callback_scope(&env);
-    Environment::AsyncHooks::ExecScope exec_scope(&env, 1, 0);
+    env.async_hooks()->push_ids(1, 0);
     LoadEnvironment(&env);
+    env.async_hooks()->pop_ids(1);
   }
 
   env.set_trace_sync_io(trace_sync_io);

From 008d3bc165be47ddc1079ad9f556797df574e815 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 8 Aug 2017 20:18:16 +0200
Subject: [PATCH 5/8] n-api: use AsyncResource for Work tracking
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Enable combining N-API async work with async-hooks.

PR-URL: https://github.com/nodejs/node/pull/14697
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>
---
 doc/api/n-api.md                              | 20 +++++++
 src/node_api.cc                               | 32 ++++++++--
 src/node_api.h                                |  2 +
 .../test_async/test-async-hooks.js            | 60 +++++++++++++++++++
 test/addons-napi/test_async/test.js           |  4 +-
 test/addons-napi/test_async/test_async.cc     | 31 ++++++----
 6 files changed, 129 insertions(+), 20 deletions(-)
 create mode 100644 test/addons-napi/test_async/test-async-hooks.js

diff --git a/doc/api/n-api.md b/doc/api/n-api.md
index b9d06079fdba19..0a74d89ea012e0 100644
--- a/doc/api/n-api.md
+++ b/doc/api/n-api.md
@@ -3256,10 +3256,16 @@ callback invocation, even when it was cancelled.
 ### napi_create_async_work
 <!-- YAML
 added: v8.0.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/14697
+    description: Added `async_resource` and `async_resource_name` parameters.
 -->
 ```C
 NAPI_EXTERN
 napi_status napi_create_async_work(napi_env env,
+                                   napi_value async_resource,
+                                   const char* async_resource_name,
                                    napi_async_execute_callback execute,
                                    napi_async_complete_callback complete,
                                    void* data,
@@ -3267,6 +3273,10 @@ napi_status napi_create_async_work(napi_env env,
 ```
 
 - `[in] env`: The environment that the API is invoked under.
+- `[in] async_resource`: An optional object associated with the async work
+  that will be passed to possible async_hooks [`init` hooks][].
+- `[in] async_resource_name`: An identifier for the kind of resource that is
+being provided for diagnostic information exposed by the `async_hooks` API.
 - `[in] execute`: The native function which should be called to excute
 the logic asynchronously.
 - `[in] complete`: The native function which will be called when the
@@ -3282,6 +3292,14 @@ This API allocates a work object that is used to execute logic asynchronously.
 It should be freed using [`napi_delete_async_work`][] once the work is no longer
 required.
 
+`async_resource_name` should be a null-terminated, UTF-8-encoded string.
+
+*Note*: The `async_resource_name` identifier is provided by the user and should
+be representative of the type of async work being performed. It is also
+recommended to apply namespacing to the identifier, e.g. by including the
+module name. See the [`async_hooks` documentation][async_hooks `type`]
+for more information.
+
 ### napi_delete_async_work
 <!-- YAML
 added: v8.0.0
@@ -3636,3 +3654,5 @@ NAPI_EXTERN napi_status napi_run_script(napi_env env,
 [`napi_wrap`]: #n_api_napi_wrap
 
 [`process.release`]: process.html#process_process_release
+[`init` hooks]: async_hooks.html#async_hooks_init_asyncid_type_triggerasyncid_resource
+[async_hooks `type`]: async_hooks.html#async_hooks_type
diff --git a/src/node_api.cc b/src/node_api.cc
index 8c55e64b7cda05..f49dd6da68669f 100644
--- a/src/node_api.cc
+++ b/src/node_api.cc
@@ -3246,13 +3246,18 @@ static napi_status ConvertUVErrorCode(int code) {
 }
 
 // Wrapper around uv_work_t which calls user-provided callbacks.
-class Work {
+class Work : public node::AsyncResource {
  private:
   explicit Work(napi_env env,
-                napi_async_execute_callback execute = nullptr,
+                v8::Local<v8::Object> async_resource,
+                const char* async_resource_name,
+                napi_async_execute_callback execute,
                 napi_async_complete_callback complete = nullptr,
                 void* data = nullptr)
-    : _env(env),
+    : AsyncResource(env->isolate,
+                    async_resource,
+                    async_resource_name),
+    _env(env),
     _data(data),
     _execute(execute),
     _complete(complete) {
@@ -3264,10 +3269,13 @@ class Work {
 
  public:
   static Work* New(napi_env env,
+                   v8::Local<v8::Object> async_resource,
+                   const char* async_resource_name,
                    napi_async_execute_callback execute,
                    napi_async_complete_callback complete,
                    void* data) {
-    return new Work(env, execute, complete, data);
+    return new Work(env, async_resource, async_resource_name,
+                    execute, complete, data);
   }
 
   static void Delete(Work* work) {
@@ -3288,6 +3296,7 @@ class Work {
       // Establish a handle scope here so that every callback doesn't have to.
       // Also it is needed for the exception-handling below.
       v8::HandleScope scope(env->isolate);
+      CallbackScope callback_scope(work);
 
       work->_complete(env, ConvertUVErrorCode(status), work->_data);
 
@@ -3330,6 +3339,8 @@ class Work {
   } while (0)
 
 napi_status napi_create_async_work(napi_env env,
+                                   napi_value async_resource,
+                                   const char* async_resource_name,
                                    napi_async_execute_callback execute,
                                    napi_async_complete_callback complete,
                                    void* data,
@@ -3338,7 +3349,18 @@ napi_status napi_create_async_work(napi_env env,
   CHECK_ARG(env, execute);
   CHECK_ARG(env, result);
 
-  uvimpl::Work* work = uvimpl::Work::New(env, execute, complete, data);
+  v8::Local<v8::Object> resource;
+  if (async_resource != nullptr) {
+    auto value = v8impl::V8LocalValueFromJsValue(async_resource);
+    RETURN_STATUS_IF_FALSE(env, value->IsObject(), napi_invalid_arg);
+    resource = value.As<v8::Object>();
+  } else {
+    resource = v8::Object::New(env->isolate);
+  }
+
+  uvimpl::Work* work =
+      uvimpl::Work::New(env, resource, async_resource_name,
+                        execute, complete, data);
 
   *result = reinterpret_cast<napi_async_work>(work);
 
diff --git a/src/node_api.h b/src/node_api.h
index 807595777dd947..227f26a2ffb6fb 100644
--- a/src/node_api.h
+++ b/src/node_api.h
@@ -524,6 +524,8 @@ NAPI_EXTERN napi_status napi_get_dataview_info(napi_env env,
 // Methods to manage simple async operations
 NAPI_EXTERN
 napi_status napi_create_async_work(napi_env env,
+                                   napi_value async_resource,
+                                   const char* async_resource_name,
                                    napi_async_execute_callback execute,
                                    napi_async_complete_callback complete,
                                    void* data,
diff --git a/test/addons-napi/test_async/test-async-hooks.js b/test/addons-napi/test_async/test-async-hooks.js
new file mode 100644
index 00000000000000..db6e0cbd1679e6
--- /dev/null
+++ b/test/addons-napi/test_async/test-async-hooks.js
@@ -0,0 +1,60 @@
+'use strict';
+const common = require('../../common');
+const assert = require('assert');
+const async_hooks = require('async_hooks');
+const test_async = require(`./build/${common.buildType}/test_async`);
+
+const events = [];
+let testId;
+const initAsyncId = async_hooks.executionAsyncId();
+
+async_hooks.createHook({
+  init(id, provider, triggerAsyncId, resource) {
+    if (provider === 'TestResource') {
+      testId = id;
+      events.push({ type: 'init', id, provider, triggerAsyncId, resource });
+    }
+  },
+  before(id) {
+    if (testId === id) {
+      events.push({ type: 'before', id });
+    }
+  },
+  after(id) {
+    if (testId === id) {
+      events.push({ type: 'after', id });
+    }
+  },
+  destroy(id) {
+    if (testId === id) {
+      events.push({ type: 'destroy', id });
+    }
+  }
+}).enable();
+
+const resource = { foo: 'foo' };
+
+events.push({ type: 'start' });
+test_async.Test(5, resource, common.mustCall(function(err, val) {
+  assert.strictEqual(err, null);
+  assert.strictEqual(val, 10);
+  events.push({ type: 'complete' });
+  process.nextTick(common.mustCall());
+}));
+events.push({ type: 'scheduled' });
+
+process.on('exit', () => {
+  assert.deepStrictEqual(events, [
+    { type: 'start' },
+    { type: 'init',
+      id: testId,
+      provider: 'TestResource',
+      triggerAsyncId: initAsyncId,
+      resource },
+    { type: 'scheduled' },
+    { type: 'before', id: testId },
+    { type: 'complete' },
+    { type: 'after', id: testId },
+    { type: 'destroy', id: testId }
+  ]);
+});
diff --git a/test/addons-napi/test_async/test.js b/test/addons-napi/test_async/test.js
index cfd39a5b11ae2d..749c48e05798a8 100644
--- a/test/addons-napi/test_async/test.js
+++ b/test/addons-napi/test_async/test.js
@@ -9,7 +9,7 @@ const testException = 'test_async_cb_exception';
 // Exception thrown from async completion callback.
 // (Tested in a spawned process because the exception is fatal.)
 if (process.argv[2] === 'child') {
-  test_async.Test(1, common.mustCall(function() {
+  test_async.Test(1, {}, common.mustCall(function() {
     throw new Error(testException);
   }));
   return;
@@ -20,7 +20,7 @@ assert.ifError(p.error);
 assert.ok(p.stderr.toString().includes(testException));
 
 // Successful async execution and completion callback.
-test_async.Test(5, common.mustCall(function(err, val) {
+test_async.Test(5, {}, common.mustCall(function(err, val) {
   assert.strictEqual(err, null);
   assert.strictEqual(val, 10);
   process.nextTick(common.mustCall());
diff --git a/test/addons-napi/test_async/test_async.cc b/test/addons-napi/test_async/test_async.cc
index fd5278d7d5cc20..4ffc1d2fc9cfac 100644
--- a/test/addons-napi/test_async/test_async.cc
+++ b/test/addons-napi/test_async/test_async.cc
@@ -61,37 +61,40 @@ void Complete(napi_env env, napi_status status, void* data) {
 
   napi_value result;
   NAPI_CALL_RETURN_VOID(env,
-    napi_make_callback(env, global, callback, 2, argv, &result));
+    napi_call_function(env, global, callback, 2, argv, &result));
 
   NAPI_CALL_RETURN_VOID(env, napi_delete_reference(env, c->_callback));
   NAPI_CALL_RETURN_VOID(env, napi_delete_async_work(env, c->_request));
 }
 
 napi_value Test(napi_env env, napi_callback_info info) {
-  size_t argc = 2;
-  napi_value argv[2];
+  size_t argc = 3;
+  napi_value argv[3];
   napi_value _this;
   void* data;
   NAPI_CALL(env,
     napi_get_cb_info(env, info, &argc, argv, &_this, &data));
-  NAPI_ASSERT(env, argc >= 2, "Not enough arguments, expected 2.");
+  NAPI_ASSERT(env, argc >= 3, "Not enough arguments, expected 2.");
 
   napi_valuetype t;
   NAPI_CALL(env, napi_typeof(env, argv[0], &t));
   NAPI_ASSERT(env, t == napi_number,
     "Wrong first argument, integer expected.");
   NAPI_CALL(env, napi_typeof(env, argv[1], &t));
+  NAPI_ASSERT(env, t == napi_object,
+    "Wrong second argument, object expected.");
+  NAPI_CALL(env, napi_typeof(env, argv[2], &t));
   NAPI_ASSERT(env, t == napi_function,
-    "Wrong second argument, function expected.");
+    "Wrong third argument, function expected.");
 
   the_carrier._output = 0;
 
   NAPI_CALL(env,
     napi_get_value_int32(env, argv[0], &the_carrier._input));
   NAPI_CALL(env,
-    napi_create_reference(env, argv[1], 1, &the_carrier._callback));
-  NAPI_CALL(env, napi_create_async_work(
-    env, Execute, Complete, &the_carrier, &the_carrier._request));
+    napi_create_reference(env, argv[2], 1, &the_carrier._callback));
+  NAPI_CALL(env, napi_create_async_work(env, argv[1], "TestResource",
+    Execute, Complete, &the_carrier, &the_carrier._request));
   NAPI_CALL(env,
     napi_queue_async_work(env, the_carrier._request));
 
@@ -116,7 +119,7 @@ void CancelComplete(napi_env env, napi_status status, void* data) {
     NAPI_CALL_RETURN_VOID(env, napi_get_global(env, &global));
     napi_value result;
     NAPI_CALL_RETURN_VOID(env,
-      napi_make_callback(env, global, callback, 0, nullptr, &result));
+      napi_call_function(env, global, callback, 0, nullptr, &result));
   }
 
   NAPI_CALL_RETURN_VOID(env, napi_delete_async_work(env, c->_request));
@@ -140,8 +143,9 @@ napi_value TestCancel(napi_env env, napi_callback_info info) {
   // make sure the work we are going to cancel will not be
   // able to start by using all the threads in the pool
   for (int i = 1; i < MAX_CANCEL_THREADS; i++) {
-    NAPI_CALL(env, napi_create_async_work(env, CancelExecute,
-      BusyCancelComplete, &async_carrier[i], &async_carrier[i]._request));
+    NAPI_CALL(env, napi_create_async_work(env, nullptr, "TestCancelBusy",
+      CancelExecute, BusyCancelComplete,
+      &async_carrier[i], &async_carrier[i]._request));
     NAPI_CALL(env, napi_queue_async_work(env, async_carrier[i]._request));
   }
 
@@ -151,8 +155,9 @@ napi_value TestCancel(napi_env env, napi_callback_info info) {
   // workers above.
   NAPI_CALL(env,
     napi_get_cb_info(env, info, &argc, argv, &_this, &data));
-  NAPI_CALL(env, napi_create_async_work(env, CancelExecute,
-    CancelComplete, &async_carrier[0], &async_carrier[0]._request));
+  NAPI_CALL(env, napi_create_async_work(env, nullptr, "TestCancelled",
+    CancelExecute, CancelComplete,
+    &async_carrier[0], &async_carrier[0]._request));
   NAPI_CALL(env,
     napi_create_reference(env, argv[0], 1, &async_carrier[0]._callback));
   NAPI_CALL(env, napi_queue_async_work(env, async_carrier[0]._request));

From 4a020253dcc5473c266915f824625206761d9e1e Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 5 Sep 2017 19:06:37 +0200
Subject: [PATCH 6/8] test: add regression test for 5691

With `CallbackScope`, this has become possible to do properly.

Fixes: https://github.com/nodejs/node/issues/5691
PR-URL: https://github.com/nodejs/node/pull/14697
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 test/addons/callback-scope/binding.cc         | 32 +++++++++++++++++++
 .../callback-scope/test-resolve-async.js      | 13 ++++++++
 2 files changed, 45 insertions(+)
 create mode 100644 test/addons/callback-scope/test-resolve-async.js

diff --git a/test/addons/callback-scope/binding.cc b/test/addons/callback-scope/binding.cc
index 31317e5dae236f..22105360fbfe5c 100644
--- a/test/addons/callback-scope/binding.cc
+++ b/test/addons/callback-scope/binding.cc
@@ -1,5 +1,6 @@
 #include "node.h"
 #include "v8.h"
+#include "uv.h"
 
 #include <assert.h>
 #include <vector>
@@ -30,8 +31,39 @@ void RunInCallbackScope(const v8::FunctionCallbackInfo<v8::Value>& args) {
     args.GetReturnValue().Set(ret.ToLocalChecked());
 }
 
+static v8::Persistent<v8::Promise::Resolver> persistent;
+
+static void Callback(uv_work_t* req, int ignored) {
+  v8::Isolate* isolate = v8::Isolate::GetCurrent();
+  v8::HandleScope scope(isolate);
+  node::CallbackScope callback_scope(isolate, v8::Object::New(isolate), {0, 0});
+
+  v8::Local<v8::Promise::Resolver> local =
+      v8::Local<v8::Promise::Resolver>::New(isolate, persistent);
+  local->Resolve(v8::Undefined(isolate));
+  delete req;
+}
+
+static void TestResolveAsync(const v8::FunctionCallbackInfo<v8::Value>& args) {
+  v8::Isolate* isolate = args.GetIsolate();
+
+  if (persistent.IsEmpty()) {
+    persistent.Reset(isolate, v8::Promise::Resolver::New(isolate));
+
+    uv_work_t* req = new uv_work_t;
+
+    uv_queue_work(uv_default_loop(), req, [](uv_work_t*) {}, Callback);
+  }
+
+  v8::Local<v8::Promise::Resolver> local =
+      v8::Local<v8::Promise::Resolver>::New(isolate, persistent);
+
+  args.GetReturnValue().Set(local->GetPromise());
+}
+
 void Initialize(v8::Local<v8::Object> exports) {
   NODE_SET_METHOD(exports, "runInCallbackScope", RunInCallbackScope);
+  NODE_SET_METHOD(exports, "testResolveAsync", TestResolveAsync);
 }
 
 }  // namespace
diff --git a/test/addons/callback-scope/test-resolve-async.js b/test/addons/callback-scope/test-resolve-async.js
new file mode 100644
index 00000000000000..e9f4b9044c0154
--- /dev/null
+++ b/test/addons/callback-scope/test-resolve-async.js
@@ -0,0 +1,13 @@
+'use strict';
+
+const common = require('../../common');
+const assert = require('assert');
+const { testResolveAsync } = require(`./build/${common.buildType}/binding`);
+
+let called = false;
+testResolveAsync().then(common.mustCall(() => {
+  called = true;
+}));
+
+setTimeout(common.mustCall(() => { assert(called); }),
+           common.platformTimeout(20));

From 733e29d47d75f854ed01c41524cb01109455f221 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Wed, 6 Sep 2017 17:49:10 +0200
Subject: [PATCH 7/8] src: move DomainEnter,DomainExit to node.cc

Reviewed-By: Trevor Norris <trev.norris@gmail.com>
---
 src/async-wrap.cc | 36 ------------------------------------
 src/async-wrap.h  |  4 ----
 src/node.cc       | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 40 deletions(-)

diff --git a/src/async-wrap.cc b/src/async-wrap.cc
index fe53d279947a96..e08b2adc3529ee 100644
--- a/src/async-wrap.cc
+++ b/src/async-wrap.cc
@@ -181,42 +181,6 @@ static void PushBackDestroyId(Environment* env, double id) {
 }
 
 
-bool DomainEnter(Environment* env, Local<Object> object) {
-  Local<Value> domain_v = object->Get(env->domain_string());
-  if (domain_v->IsObject()) {
-    Local<Object> domain = domain_v.As<Object>();
-    if (domain->Get(env->disposed_string())->IsTrue())
-      return true;
-    Local<Value> enter_v = domain->Get(env->enter_string());
-    if (enter_v->IsFunction()) {
-      if (enter_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
-        FatalError("node::AsyncWrap::MakeCallback",
-                   "domain enter callback threw, please report this");
-      }
-    }
-  }
-  return false;
-}
-
-
-bool DomainExit(Environment* env, v8::Local<v8::Object> object) {
-  Local<Value> domain_v = object->Get(env->domain_string());
-  if (domain_v->IsObject()) {
-    Local<Object> domain = domain_v.As<Object>();
-    if (domain->Get(env->disposed_string())->IsTrue())
-      return true;
-    Local<Value> exit_v = domain->Get(env->exit_string());
-    if (exit_v->IsFunction()) {
-      if (exit_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
-        FatalError("node::AsyncWrap::MakeCallback",
-                  "domain exit callback threw, please report this");
-      }
-    }
-  }
-  return false;
-}
-
-
 void AsyncWrap::EmitBefore(Environment* env, double async_id) {
   AsyncHooks* async_hooks = env->async_hooks();
 
diff --git a/src/async-wrap.h b/src/async-wrap.h
index 2b631234b15fdc..681d4fd13b9af9 100644
--- a/src/async-wrap.h
+++ b/src/async-wrap.h
@@ -156,10 +156,6 @@ class AsyncWrap : public BaseObject {
 
 void LoadAsyncWrapperInfo(Environment* env);
 
-// Return value is an indicator whether the domain was disposed.
-bool DomainEnter(Environment* env, v8::Local<v8::Object> object);
-bool DomainExit(Environment* env, v8::Local<v8::Object> object);
-
 }  // namespace node
 
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
diff --git a/src/node.cc b/src/node.cc
index 3cfaf8d12944c8..8999014713d21a 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -1162,6 +1162,42 @@ bool ShouldAbortOnUncaughtException(Isolate* isolate) {
 }
 
 
+bool DomainEnter(Environment* env, Local<Object> object) {
+  Local<Value> domain_v = object->Get(env->domain_string());
+  if (domain_v->IsObject()) {
+    Local<Object> domain = domain_v.As<Object>();
+    if (domain->Get(env->disposed_string())->IsTrue())
+      return true;
+    Local<Value> enter_v = domain->Get(env->enter_string());
+    if (enter_v->IsFunction()) {
+      if (enter_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
+        FatalError("node::AsyncWrap::MakeCallback",
+                   "domain enter callback threw, please report this");
+      }
+    }
+  }
+  return false;
+}
+
+
+bool DomainExit(Environment* env, v8::Local<v8::Object> object) {
+  Local<Value> domain_v = object->Get(env->domain_string());
+  if (domain_v->IsObject()) {
+    Local<Object> domain = domain_v.As<Object>();
+    if (domain->Get(env->disposed_string())->IsTrue())
+      return true;
+    Local<Value> exit_v = domain->Get(env->exit_string());
+    if (exit_v->IsFunction()) {
+      if (exit_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
+        FatalError("node::AsyncWrap::MakeCallback",
+                  "domain exit callback threw, please report this");
+      }
+    }
+  }
+  return false;
+}
+
+
 void DomainPromiseHook(PromiseHookType type,
                        Local<Promise> promise,
                        Local<Value> parent,

From 15aca603437e6f4a3916cf4c92b8ed525ac5ad0e Mon Sep 17 00:00:00 2001
From: Jason Ginchereau <jasongin@microsoft.com>
Date: Sat, 9 Sep 2017 14:04:35 -0700
Subject: [PATCH 8/8] n-api: change async resource name to napi_value

Reviewed-By: Anna Henningsen <anna@addaleax.net>
---
 doc/api/n-api.md                          |  2 +-
 src/async-wrap.cc                         | 15 +++++++++++----
 src/node.h                                | 15 +++++++++++++++
 src/node_api.cc                           | 17 ++++++++++-------
 src/node_api.h                            |  2 +-
 test/addons-napi/test_async/test_async.cc | 14 +++++++++++---
 6 files changed, 49 insertions(+), 16 deletions(-)

diff --git a/doc/api/n-api.md b/doc/api/n-api.md
index 0a74d89ea012e0..b827367bc469b9 100644
--- a/doc/api/n-api.md
+++ b/doc/api/n-api.md
@@ -3265,7 +3265,7 @@ changes:
 NAPI_EXTERN
 napi_status napi_create_async_work(napi_env env,
                                    napi_value async_resource,
-                                   const char* async_resource_name,
+                                   napi_value async_resource_name,
                                    napi_async_execute_callback execute,
                                    napi_async_complete_callback complete,
                                    void* data,
diff --git a/src/async-wrap.cc b/src/async-wrap.cc
index e08b2adc3529ee..b678f1500bb47f 100644
--- a/src/async-wrap.cc
+++ b/src/async-wrap.cc
@@ -643,6 +643,16 @@ async_context EmitAsyncInit(Isolate* isolate,
                             Local<Object> resource,
                             const char* name,
                             async_id trigger_async_id) {
+  Local<String> type =
+      String::NewFromUtf8(isolate, name, v8::NewStringType::kInternalized)
+          .ToLocalChecked();
+  return EmitAsyncInit(isolate, resource, type, trigger_async_id);
+}
+
+async_context EmitAsyncInit(Isolate* isolate,
+                            Local<Object> resource,
+                            v8::Local<v8::String> name,
+                            async_id trigger_async_id) {
   Environment* env = Environment::GetCurrent(isolate);
 
   // Initialize async context struct
@@ -655,10 +665,7 @@ async_context EmitAsyncInit(Isolate* isolate,
   };
 
   // Run init hooks
-  Local<String> type =
-      String::NewFromUtf8(isolate, name, v8::NewStringType::kInternalized)
-          .ToLocalChecked();
-  AsyncWrap::EmitAsyncInit(env, resource, type, context.async_id,
+  AsyncWrap::EmitAsyncInit(env, resource, name, context.async_id,
                            context.trigger_async_id);
 
   return context;
diff --git a/src/node.h b/src/node.h
index 6558a9ee6da96e..8d93fea7dc4bd8 100644
--- a/src/node.h
+++ b/src/node.h
@@ -566,6 +566,11 @@ NODE_EXTERN async_context EmitAsyncInit(v8::Isolate* isolate,
                                         const char* name,
                                         async_id trigger_async_id = -1);
 
+NODE_EXTERN async_context EmitAsyncInit(v8::Isolate* isolate,
+                                        v8::Local<v8::Object> resource,
+                                        v8::Local<v8::String> name,
+                                        async_id trigger_async_id = -1);
+
 /* Emit the destroy() callback. */
 NODE_EXTERN void EmitAsyncDestroy(v8::Isolate* isolate,
                                   async_context asyncContext);
@@ -647,6 +652,16 @@ class AsyncResource {
                                      trigger_async_id);
     }
 
+    AsyncResource(v8::Isolate* isolate,
+                  v8::Local<v8::Object> resource,
+                  v8::Local<v8::String> name,
+                  async_id trigger_async_id = -1)
+        : isolate_(isolate),
+          resource_(isolate, resource) {
+      async_context_ = EmitAsyncInit(isolate, resource, name,
+                                     trigger_async_id);
+    }
+
     ~AsyncResource() {
       EmitAsyncDestroy(isolate_, async_context_);
     }
diff --git a/src/node_api.cc b/src/node_api.cc
index f49dd6da68669f..8f84de7879ddde 100644
--- a/src/node_api.cc
+++ b/src/node_api.cc
@@ -3250,7 +3250,7 @@ class Work : public node::AsyncResource {
  private:
   explicit Work(napi_env env,
                 v8::Local<v8::Object> async_resource,
-                const char* async_resource_name,
+                v8::Local<v8::String> async_resource_name,
                 napi_async_execute_callback execute,
                 napi_async_complete_callback complete = nullptr,
                 void* data = nullptr)
@@ -3270,7 +3270,7 @@ class Work : public node::AsyncResource {
  public:
   static Work* New(napi_env env,
                    v8::Local<v8::Object> async_resource,
-                   const char* async_resource_name,
+                   v8::Local<v8::String> async_resource_name,
                    napi_async_execute_callback execute,
                    napi_async_complete_callback complete,
                    void* data) {
@@ -3340,7 +3340,7 @@ class Work : public node::AsyncResource {
 
 napi_status napi_create_async_work(napi_env env,
                                    napi_value async_resource,
-                                   const char* async_resource_name,
+                                   napi_value async_resource_name,
                                    napi_async_execute_callback execute,
                                    napi_async_complete_callback complete,
                                    void* data,
@@ -3349,17 +3349,20 @@ napi_status napi_create_async_work(napi_env env,
   CHECK_ARG(env, execute);
   CHECK_ARG(env, result);
 
+  v8::Local<v8::Context> context = env->isolate->GetCurrentContext();
+
   v8::Local<v8::Object> resource;
   if (async_resource != nullptr) {
-    auto value = v8impl::V8LocalValueFromJsValue(async_resource);
-    RETURN_STATUS_IF_FALSE(env, value->IsObject(), napi_invalid_arg);
-    resource = value.As<v8::Object>();
+    CHECK_TO_OBJECT(env, context, resource, async_resource);
   } else {
     resource = v8::Object::New(env->isolate);
   }
 
+  v8::Local<v8::String> resource_name;
+  CHECK_TO_STRING(env, context, resource_name, async_resource_name);
+
   uvimpl::Work* work =
-      uvimpl::Work::New(env, resource, async_resource_name,
+      uvimpl::Work::New(env, resource, resource_name,
                         execute, complete, data);
 
   *result = reinterpret_cast<napi_async_work>(work);
diff --git a/src/node_api.h b/src/node_api.h
index 227f26a2ffb6fb..b127fd7fe80070 100644
--- a/src/node_api.h
+++ b/src/node_api.h
@@ -525,7 +525,7 @@ NAPI_EXTERN napi_status napi_get_dataview_info(napi_env env,
 NAPI_EXTERN
 napi_status napi_create_async_work(napi_env env,
                                    napi_value async_resource,
-                                   const char* async_resource_name,
+                                   napi_value async_resource_name,
                                    napi_async_execute_callback execute,
                                    napi_async_complete_callback complete,
                                    void* data,
diff --git a/test/addons-napi/test_async/test_async.cc b/test/addons-napi/test_async/test_async.cc
index 4ffc1d2fc9cfac..574785c6d1ebb4 100644
--- a/test/addons-napi/test_async/test_async.cc
+++ b/test/addons-napi/test_async/test_async.cc
@@ -71,6 +71,7 @@ napi_value Test(napi_env env, napi_callback_info info) {
   size_t argc = 3;
   napi_value argv[3];
   napi_value _this;
+  napi_value resource_name;
   void* data;
   NAPI_CALL(env,
     napi_get_cb_info(env, info, &argc, argv, &_this, &data));
@@ -93,7 +94,10 @@ napi_value Test(napi_env env, napi_callback_info info) {
     napi_get_value_int32(env, argv[0], &the_carrier._input));
   NAPI_CALL(env,
     napi_create_reference(env, argv[2], 1, &the_carrier._callback));
-  NAPI_CALL(env, napi_create_async_work(env, argv[1], "TestResource",
+
+  NAPI_CALL(env,
+    napi_create_string_utf8(env, "TestResource", -1, &resource_name));
+  NAPI_CALL(env, napi_create_async_work(env, argv[1], resource_name,
     Execute, Complete, &the_carrier, &the_carrier._request));
   NAPI_CALL(env,
     napi_queue_async_work(env, the_carrier._request));
@@ -138,12 +142,16 @@ napi_value TestCancel(napi_env env, napi_callback_info info) {
   size_t argc = 1;
   napi_value argv[1];
   napi_value _this;
+  napi_value resource_name;
   void* data;
 
+  NAPI_CALL(env,
+    napi_create_string_utf8(env, "TestResource", -1, &resource_name));
+
   // make sure the work we are going to cancel will not be
   // able to start by using all the threads in the pool
   for (int i = 1; i < MAX_CANCEL_THREADS; i++) {
-    NAPI_CALL(env, napi_create_async_work(env, nullptr, "TestCancelBusy",
+    NAPI_CALL(env, napi_create_async_work(env, nullptr, resource_name,
       CancelExecute, BusyCancelComplete,
       &async_carrier[i], &async_carrier[i]._request));
     NAPI_CALL(env, napi_queue_async_work(env, async_carrier[i]._request));
@@ -155,7 +163,7 @@ napi_value TestCancel(napi_env env, napi_callback_info info) {
   // workers above.
   NAPI_CALL(env,
     napi_get_cb_info(env, info, &argc, argv, &_this, &data));
-  NAPI_CALL(env, napi_create_async_work(env, nullptr, "TestCancelled",
+  NAPI_CALL(env, napi_create_async_work(env, nullptr, resource_name,
     CancelExecute, CancelComplete,
     &async_carrier[0], &async_carrier[0]._request));
   NAPI_CALL(env,