src: handle microtask/nextTick draining in coroutines

jasnell · jasnell · commit 12e85b00a61e · 2026-03-29T18:02:15.000-07:00
diff --git a/src/coro/uv_promise.h b/src/coro/uv_promise.h
@@ -1,55 +1,35 @@
 #ifndef SRC_CORO_UV_PROMISE_H_
 #define SRC_CORO_UV_PROMISE_H_
 
-#include "coro/uv_task.h"
 #include "env-inl.h"
 #include "node_errors.h"
 #include "v8.h"
 
 namespace node {
 namespace coro {
 
-// ---------------------------------------------------------------------------
-// CoroutineScope — RAII helper for V8 scope management inside coroutines.
-//
-// After co_await returns, we're back on the event loop thread but may
-// not have an active HandleScope or Context.  This sets them up:
-//
-//   CoroutineScope scope(env);
-//   auto value = v8::Integer::New(scope.isolate(), 42);
-//   resolver.Get(scope.isolate())->Resolve(scope.context(), value);
-// ---------------------------------------------------------------------------
-
-class CoroutineScope {
- public:
-  explicit CoroutineScope(Environment* env)
-      : handle_scope_(env->isolate()), context_scope_(env->context()) {}
-
-  v8::Isolate* isolate() const { return handle_scope_.GetIsolate(); }
-
- private:
-  v8::HandleScope handle_scope_;
-  v8::Context::Scope context_scope_;
-
-  // Access context via the environment to avoid storing it
-};
-
 // ---------------------------------------------------------------------------
 // ResolvePromise / RejectPromise — helpers for resolving/rejecting a
 // v8::Global<v8::Promise::Resolver> from inside a coroutine.
+//
+// When used inside a UvTrackedTask, the InternalCallbackScope from
+// on_resume() already provides a HandleScope and will drain microtasks
+// and nextTick on close (in on_suspend / final_suspend).
+//
+// When used inside an untracked UvTask (no InternalCallbackScope),
+// these create their own HandleScope.
 // ---------------------------------------------------------------------------
 
 inline void ResolvePromise(Environment* env,
                            v8::Global<v8::Promise::Resolver>& resolver,
                            v8::Local<v8::Value> value) {
-  CoroutineScope scope(env);
+  v8::HandleScope scope(env->isolate());
   auto local = resolver.Get(env->isolate());
   USE(local->Resolve(env->context(), value));
 }
 
 inline void ResolvePromiseUndefined(
     Environment* env, v8::Global<v8::Promise::Resolver>& resolver) {
-  CoroutineScope scope(env);
   ResolvePromise(env, resolver, v8::Undefined(env->isolate()));
 }
 
@@ -59,7 +39,7 @@ inline void RejectPromiseWithUVError(
     int uv_err,
     const char* syscall,
     const char* path = nullptr) {
-  CoroutineScope scope(env);
+  v8::HandleScope scope(env->isolate());
   auto local = resolver.Get(env->isolate());
   v8::Local<v8::Value> exception =
       UVException(env->isolate(), uv_err, syscall, nullptr, path);
diff --git a/src/coro/uv_tracked_task.h b/src/coro/uv_tracked_task.h
@@ -12,39 +12,36 @@
 #include "async_context_frame.h"
 #include "async_wrap.h"
 #include "env-inl.h"
+#include "node_internals.h"
 #include "v8.h"
 
 #include "coro/uv_task.h"  // for detail::PromiseBase
 
 namespace node {
 namespace coro {
 
-// ---------------------------------------------------------------------------
-// FixedString — a structural type that can be used as a non-type template
-// parameter in C++20.  Allows compile-time string literals:
-//
-//   UvTrackedTask<void, "FSREQPROMISE"> my_coro(...) { ... }
-//
-// The implicit deduction guide lets the string literal deduce N.
-// ---------------------------------------------------------------------------
-
 template <size_t N>
-struct FixedString {
+struct ConstString {
   char data[N]{};
-  consteval FixedString(const char (&str)[N]) {  // NOLINT(runtime/explicit)
+  consteval ConstString(const char (&str)[N]) {  // NOLINT(runtime/explicit)
     std::copy_n(str, N, data);
   }
   constexpr const char* c_str() const { return data; }
   constexpr size_t size() const { return N - 1; }  // exclude null terminator
 };
 
-// Deduction guide: FixedString("hello") deduces N = 6.
+// Deduction guide: ConstString("hello") deduces N = 6.
 template <size_t N>
-FixedString(const char (&)[N]) -> FixedString<N>;
+ConstString(const char (&)[N]) -> ConstString<N>;
 
 // ---------------------------------------------------------------------------
-// UvTrackedTask<T, Name> — A coroutine return type with full async_hooks
-// and async_context_frame integration.
+// UvTrackedTask<T, Name> — A coroutine return type with full async_hooks,
+// async_context_frame, and microtask/nextTick draining integration.
+//
+// Each resume-to-suspend segment of the coroutine is wrapped in an
+// InternalCallbackScope, giving it the same semantics as any other
+// callback entry into Node.js: async_hooks before/after, context frame
+// save/restore, and microtask+nextTick draining on close.
 //
 // The Name template parameter identifies the async resource type visible
 // to async_hooks.createHook() init callbacks — analogous to the per-
@@ -62,14 +59,13 @@ FixedString(const char (&)[N]) -> FixedString<N>;
 //   task.InitTracking(env);
 //   task.Start();
 // ---------------------------------------------------------------------------
-
-template <typename T, FixedString Name>
+template <typename T, ConstString Name>
 class UvTrackedTask;
 
 namespace detail {
 
-// TrackedAwaitable — wraps any awaitable with before/after hook emission
-// and async_context_frame save/restore.  Created by await_transform.
+// TrackedAwaitable — wraps any awaitable with InternalCallbackScope
+// management.  Created by await_transform.
 template <typename Inner, typename Promise>
 struct TrackedAwaitable {
   Inner inner_;
@@ -78,34 +74,50 @@ struct TrackedAwaitable {
   bool await_ready() noexcept { return inner_.await_ready(); }
 
   // Called when the coroutine is about to suspend.
+  // Close the InternalCallbackScope — this emits after hooks,
+  // restores the async context, and drains microtasks + nextTick.
   template <typename H>
   auto await_suspend(H h) noexcept {
     promise_->on_suspend();
     return inner_.await_suspend(h);
   }
 
   // Called when the coroutine resumes after the async operation.
+  // Open a new InternalCallbackScope — this emits before hooks,
+  // pushes the async context, and swaps in the context frame.
   auto await_resume() noexcept(noexcept(inner_.await_resume())) {
     promise_->on_resume();
     return inner_.await_resume();
   }
 };
 
 // TrackedPromiseBase — extends PromiseBase with async_hooks state.
+// Each resume-to-suspend segment is bracketed by an InternalCallbackScope,
+// which handles all async context management and task queue draining.
 struct TrackedPromiseBase : PromiseBase {
   Environment* env_ = nullptr;
   double async_id_ = -1;
   double trigger_async_id_ = -1;
+
+  // The async_context_frame captured at init — passed to each scope.
   v8::Global<v8::Value> context_frame_;
-  v8::Global<v8::Value> prior_context_frame_;
 
-  // Whether any async_hooks are active (checked once at init).
-  bool hooks_active_ = false;
+  // The resource object — created once at init, reused for each scope
+  // and for executionAsyncResource().
+  v8::Global<v8::Object> resource_;
 
-  // Track whether we're inside a resume (to avoid double-pop on final).
-  bool in_resume_ = false;
+  // HandleScope that lives across the entire resume-to-suspend segment.
+  // Must be created before and destroyed after the InternalCallbackScope.
+  std::optional<v8::HandleScope> handle_scope_;
+
+  // The active InternalCallbackScope for the current resume segment.
+  // Created in on_resume(), closed and destroyed in on_suspend().
+  std::optional<InternalCallbackScope> scope_;
 
   // Called by InitTracking() on the task.
+  // `type_name` identifies the subsystem (e.g. "FSREQPROMISE",
+  // "GETADDRINFOREQWRAP") — analogous to AsyncWrap::ProviderType but
+  // as a free-form string so callers aren't coupled to the enum.
   void init_tracking(Environment* env, const char* type_name) {
     env_ = env;
     v8::Isolate* isolate = env->isolate();
@@ -118,76 +130,62 @@ struct TrackedPromiseBase : PromiseBase {
     // Capture the current async_context_frame (for AsyncLocalStorage)
     context_frame_.Reset(isolate, async_context_frame::current(isolate));
 
-    // Check if any hooks are active
-    AsyncHooks* hooks = env->async_hooks();
-    hooks_active_ = hooks->fields()[AsyncHooks::kInit] > 0 ||
-                    hooks->fields()[AsyncHooks::kBefore] > 0 ||
-                    hooks->fields()[AsyncHooks::kAfter] > 0 ||
-                    hooks->fields()[AsyncHooks::kDestroy] > 0;
+    // Create the resource object (reused across scopes)
+    resource_.Reset(isolate, v8::Object::New(isolate));
 
     // Emit init hook with the caller-supplied type name
-    if (hooks_active_ && hooks->fields()[AsyncHooks::kInit] > 0) {
-      v8::Local<v8::Object> resource = v8::Object::New(isolate);
+    AsyncHooks* hooks = env->async_hooks();
+    if (hooks->fields()[AsyncHooks::kInit] > 0) {
       v8::Local<v8::String> type =
           v8::String::NewFromUtf8(isolate, type_name).ToLocalChecked();
       AsyncWrap::EmitAsyncInit(
-          env, resource, type, async_id_, trigger_async_id_);
+          env, resource_.Get(isolate), type, async_id_, trigger_async_id_);
     }
   }
 
   // Called when the coroutine resumes (at start or after co_await).
+  // Opens an InternalCallbackScope which:
+  //   - Pushes async callback depth
+  //   - Swaps in the context frame (for AsyncLocalStorage)
+  //   - Pushes async context (async_id, trigger_async_id)
+  //   - Emits before hook
   void on_resume() noexcept {
     if (env_ == nullptr) return;
-    in_resume_ = true;
-
-    v8::Isolate* isolate = env_->isolate();
-    v8::HandleScope handle_scope(isolate);
 
-    // Phase 1: Restore async_context_frame (for AsyncLocalStorage)
-    prior_context_frame_.Reset(
-        isolate,
-        async_context_frame::exchange(isolate, context_frame_.Get(isolate)));
-
-    // Phase 2: async_hooks before
-    if (hooks_active_) {
-      env_->async_hooks()->push_async_context(
-          async_id_,
-          trigger_async_id_,
-          static_cast<v8::Local<v8::Object>*>(nullptr));
-      AsyncWrap::EmitBefore(env_, async_id_);
-    }
+    // HandleScope must be created BEFORE and destroyed AFTER the
+    // InternalCallbackScope — matching the pattern in FSReqPromise::Resolve.
+    handle_scope_.emplace(env_->isolate());
+    scope_.emplace(env_,
+                   &resource_,
+                   async_context{async_id_, trigger_async_id_},
+                   InternalCallbackScope::kNoFlags,
+                   context_frame_.Get(env_->isolate()));
   }
 
   // Called when the coroutine suspends (at co_await or completion).
+  // Closes and destroys the InternalCallbackScope which:
+  //   - Emits after hook
+  //   - Pops async context
+  //   - Restores prior context frame
+  //   - At outermost depth: drains microtasks and nextTick queue
+  //   - Pops async callback depth
   void on_suspend() noexcept {
-    if (env_ == nullptr || !in_resume_) return;
-    in_resume_ = false;
-
-    v8::Isolate* isolate = env_->isolate();
-    v8::HandleScope handle_scope(isolate);
-
-    // Phase 2: async_hooks after
-    if (hooks_active_) {
-      AsyncWrap::EmitAfter(env_, async_id_);
-      env_->async_hooks()->pop_async_context(async_id_);
-    }
-
-    // Phase 1: Restore prior async_context_frame
-    async_context_frame::set(isolate, prior_context_frame_.Get(isolate));
+    if (!scope_.has_value()) return;
+    scope_->Close();
+    scope_.reset();         // ~InternalCallbackScope (pops callback depth)
+    handle_scope_.reset();  // ~HandleScope (must be after scope_)
   }
 
   // Called when the coroutine is done (at final_suspend).
   void on_destroy() noexcept {
     if (env_ == nullptr) return;
 
     // Emit destroy hook (batched by Node.js internals)
-    if (hooks_active_) {
-      AsyncWrap::EmitDestroy(env_, async_id_);
-    }
+    AsyncWrap::EmitDestroy(env_, async_id_);
 
     // Release globals
     context_frame_.Reset();
-    prior_context_frame_.Reset();
+    resource_.Reset();
   }
 };
 
@@ -199,18 +197,17 @@ struct TrackedInitialAwaiter {
   bool await_ready() noexcept { return false; }
   void await_suspend(std::coroutine_handle<>) noexcept {}
 
-  // When resumed, fire on_resume to set up async context.
+  // When resumed, open the first InternalCallbackScope.
   void await_resume() noexcept { promise_.on_resume(); }
 
   Promise& promise_;
 };
 
-// FinalAwaiter for tracked tasks — emits on_suspend + on_destroy,
+// FinalAwaiter for tracked tasks — closes the last scope, emits destroy,
 // then handles the standard detach/continuation logic.
 template <typename Promise>
 struct TrackedFinalAwaiter {
   bool await_ready() noexcept {
-    // Emit hooks before deciding whether to suspend.
     promise_.on_suspend();
     promise_.on_destroy();
     return promise_.detached_;
@@ -232,7 +229,7 @@ struct TrackedFinalAwaiter {
 // ===========================================================================
 // UvTrackedTask<T, Name> — tracked coroutine that produces a T value
 // ===========================================================================
-template <typename T, FixedString Name>
+template <typename T, ConstString Name>
 class UvTrackedTask {
  public:
   struct promise_type : detail::TrackedPromiseBase {
@@ -253,7 +250,6 @@ class UvTrackedTask {
 
     void return_value(T value) { result_.emplace(std::move(value)); }
 
-    // await_transform — intercepts every co_await to wrap with tracking.
     template <typename Awaitable>
     auto await_transform(Awaitable&& aw) {
       return detail::TrackedAwaitable<std::decay_t<Awaitable>, promise_type>{
@@ -277,7 +273,6 @@ class UvTrackedTask {
   }
 
   // -- Tracking setup --
-  // The type name comes from the template parameter — no string arg needed.
 
   void InitTracking(Environment* env) {
     handle_.promise().init_tracking(env, Name.c_str());
@@ -320,7 +315,7 @@ class UvTrackedTask {
 // ===========================================================================
 // Partial specialization for T = void
 // ===========================================================================
-template <FixedString Name>
+template <ConstString Name>
 class UvTrackedTask<void, Name> {
  public:
   struct promise_type : detail::TrackedPromiseBase {
diff --git a/src/node_file.cc b/src/node_file.cc
@@ -4206,15 +4206,15 @@ static coro::UvTrackedTask<void, "COROREADFILE"> CoroReadFileBytesImpl(
   // 4. Close the file
   co_await coro::UvFs(env->event_loop(), uv_fs_close, static_cast<uv_file>(fd));
 
-  // 5. Create a Buffer and resolve the promise
+  // 5. Create a Buffer and resolve the promise.
+  // The InternalCallbackScope from on_resume() provides the HandleScope.
+  // Microtask and nextTick draining happens in on_suspend() via Close().
   {
-    coro::CoroutineScope scope(env);
     MaybeLocal<Object> maybe_buf = Buffer::Copy(env, data.data(), total_read);
     Local<Object> buf;
     if (maybe_buf.ToLocal(&buf)) {
       coro::ResolvePromise(env, resolver, buf);
     } else {
-      // Buffer allocation failed — reject with an error
       auto msg = String::NewFromUtf8Literal(env->isolate(),
                                             "Failed to allocate buffer");
       auto err = v8::Exception::Error(msg);
