n-api: back up env before async work finalize

We must back up the value of `_env` before calling the async work
complete callback, because the complete callback may delete the
instance in which `_env` is stored by calling `napi_delete_async_work`,
and because we need to use it after the complete callback has
completed.

Fixes: #20966
PR-URL: #21129
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>

src/node_api.cc

@@ -3393,13 +3393,19 @@ class Work : public node::AsyncResource, public node::ThreadPoolWork {
 
     CallbackScope callback_scope(this);
 
-    NAPI_CALL_INTO_MODULE(_env,
+    // We have to back up the env here because the `NAPI_CALL_INTO_MODULE` macro
+    // makes use of it after the call into the module completes, but the module
+    // may have deallocated **this**, and along with it the place where _env is
+    // stored.
+    napi_env env = _env;
+
+    NAPI_CALL_INTO_MODULE(env,
         _complete(_env, ConvertUVErrorCode(status), _data),
-        [this] (v8::Local<v8::Value> local_err) {
+        [env] (v8::Local<v8::Value> local_err) {
           // If there was an unhandled exception in the complete callback,
           // report it as a fatal exception. (There is no JavaScript on the
           // callstack that can possibly handle it.)
-          v8impl::trigger_fatal_exception(_env, local_err);
+          v8impl::trigger_fatal_exception(env, local_err);
         });
 
     // Note: Don't access `work` after this point because it was

test/addons-napi/test_async/test-loop.js

@@ -0,0 +1,14 @@
+'use strict';
+const common = require('../../common');
+const assert = require('assert');
+const test_async = require(`./build/${common.buildType}/test_async`);
+const iterations = 500;
+
+let x = 0;
+const workDone = common.mustCall((status) => {
+  assert.strictEqual(status, 0, 'Work completed successfully');
+  if (++x < iterations) {
+    setImmediate(() => test_async.DoRepeatedWork(workDone));
+  }
+}, iterations);
+test_async.DoRepeatedWork(workDone);

test/addons-napi/test_async/test_async.cc

@@ -1,3 +1,4 @@
+#include <stdio.h>
 #include <node_api.h>
 #include "../common.h"
 
@@ -173,10 +174,52 @@ napi_value TestCancel(napi_env env, napi_callback_info info) {
   return nullptr;
 }
 
+struct {
+  napi_ref ref;
+  napi_async_work work;
+} repeated_work_info = { nullptr, nullptr };
+
+static void RepeatedWorkerThread(napi_env env, void* data) {}
+
+static void RepeatedWorkComplete(napi_env env, napi_status status, void* data) {
+  napi_value cb, js_status;
+  NAPI_CALL_RETURN_VOID(env,
+      napi_get_reference_value(env, repeated_work_info.ref, &cb));
+  NAPI_CALL_RETURN_VOID(env,
+      napi_delete_async_work(env, repeated_work_info.work));
+  NAPI_CALL_RETURN_VOID(env,
+      napi_delete_reference(env, repeated_work_info.ref));
+  repeated_work_info.work = nullptr;
+  repeated_work_info.ref = nullptr;
+  NAPI_CALL_RETURN_VOID(env,
+      napi_create_uint32(env, (uint32_t)status, &js_status));
+  NAPI_CALL_RETURN_VOID(env,
+      napi_call_function(env, cb, cb, 1, &js_status, nullptr));
+}
+
+static napi_value DoRepeatedWork(napi_env env, napi_callback_info info) {
+  size_t argc = 1;
+  napi_value cb, name;
+  NAPI_ASSERT(env, repeated_work_info.ref == nullptr,
+      "Reference left over from previous work");
+  NAPI_ASSERT(env, repeated_work_info.work == nullptr,
+      "Work pointer left over from previous work");
+  NAPI_CALL(env, napi_get_cb_info(env, info, &argc, &cb, nullptr, nullptr));
+  NAPI_CALL(env, napi_create_reference(env, cb, 1, &repeated_work_info.ref));
+  NAPI_CALL(env,
+      napi_create_string_utf8(env, "Repeated Work", NAPI_AUTO_LENGTH, &name));
+  NAPI_CALL(env,
+      napi_create_async_work(env, nullptr, name, RepeatedWorkerThread,
+          RepeatedWorkComplete, &repeated_work_info, &repeated_work_info.work));
+  NAPI_CALL(env, napi_queue_async_work(env, repeated_work_info.work));
+  return nullptr;
+}
+
 napi_value Init(napi_env env, napi_value exports) {
   napi_property_descriptor properties[] = {
     DECLARE_NAPI_PROPERTY("Test", Test),
     DECLARE_NAPI_PROPERTY("TestCancel", TestCancel),
+    DECLARE_NAPI_PROPERTY("DoRepeatedWork", DoRepeatedWork),
   };
 
   NAPI_CALL(env, napi_define_properties(