src: add AsyncWorker destruction suppression

Add method `SuppressDestruct()` to `AsyncWorker`, which will cause an
instance of the class to remain allocated even after the `OnOK`
callback fires. Such an instance must be explicitly `delete`-ed from
user code.

Re: #231
Re: nodejs/abi-stable-node#353

doc/async_worker.md

@@ -66,6 +66,15 @@ the computation that happened in the `Napi::AsyncWorker::Execute` method, unless
 the default implementation of `Napi::AsyncWorker::OnOK` or
 `Napi::AsyncWorker::OnError` is overridden.
 
+### SuppressDestruct
+
+```cpp
+void Napi::AsyncWorker::SuppressDestruct();
+```
+
+Prevents the destruction of the `Napi::AsyncWorker` instance upon completion of
+the `Napi::AsyncWorker::OnOK` callback.
+
 ### SetError
 
 Sets the error message for the error that happened during the execution. Setting

napi-inl.h

@@ -3524,7 +3524,8 @@ inline AsyncWorker::AsyncWorker(const Object& receiver,
                                 const Object& resource)
   : _env(callback.Env()),
     _receiver(Napi::Persistent(receiver)),
-    _callback(Napi::Persistent(callback)) {
+    _callback(Napi::Persistent(callback)),
+    _suppress_destruct(false) {
   napi_value resource_id;
   napi_status status = napi_create_string_latin1(
       _env, resource_name, NAPI_AUTO_LENGTH, &resource_id);
@@ -3550,6 +3551,7 @@ inline AsyncWorker::AsyncWorker(AsyncWorker&& other) {
   _receiver = std::move(other._receiver);
   _callback = std::move(other._callback);
   _error = std::move(other._error);
+  _suppress_destruct = other._suppress_destruct;
 }
 
 inline AsyncWorker& AsyncWorker::operator =(AsyncWorker&& other) {
@@ -3560,6 +3562,7 @@ inline AsyncWorker& AsyncWorker::operator =(AsyncWorker&& other) {
   _receiver = std::move(other._receiver);
   _callback = std::move(other._callback);
   _error = std::move(other._error);
+  _suppress_destruct = other._suppress_destruct;
   return *this;
 }
 
@@ -3589,6 +3592,10 @@ inline FunctionReference& AsyncWorker::Callback() {
   return _callback;
 }
 
+inline void AsyncWorker::SuppressDestruct() {
+  _suppress_destruct = true;
+}
+
 inline void AsyncWorker::OnOK() {
   _callback.Call(_receiver.Value(), std::initializer_list<napi_value>{});
 }
@@ -3629,7 +3636,9 @@ inline void AsyncWorker::OnWorkComplete(
       return nullptr;
     });
   }
-  delete self;
+  if (!self->_suppress_destruct) {
+    delete self;
+  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////

napi.h

@@ -1722,6 +1722,7 @@ namespace Napi {
 
     void Queue();
     void Cancel();
+    void SuppressDestruct();
 
     ObjectReference& Receiver();
     FunctionReference& Callback();
@@ -1760,6 +1761,7 @@ namespace Napi {
     ObjectReference _receiver;
     FunctionReference _callback;
     std::string _error;
+    bool _suppress_destruct;
   };
 
   // Memory management.

test/asyncworker-persistent.cc

@@ -0,0 +1,69 @@
+#include "napi.h"
+
+// A variant of TestWorker wherein destruction is suppressed. That is, instances
+// are not destroyed during the `OnOK` callback. They must be explicitly
+// destroyed.
+
+using namespace Napi;
+
+class PersistentTestWorker : public AsyncWorker {
+public:
+  static PersistentTestWorker* current_worker;
+  static void DoWork(const CallbackInfo& info) {
+    bool succeed = info[0].As<Boolean>();
+    Object resource = info[1].As<Object>();
+    Function cb = info[2].As<Function>();
+    Value data = info[3];
+
+    PersistentTestWorker* worker =
+        new PersistentTestWorker(cb, "TestResource", resource);
+    current_worker = worker;
+
+    worker->SuppressDestruct();
+    worker->Receiver().Set("data", data);
+    worker->_succeed = succeed;
+    worker->Queue();
+  }
+
+  static Value GetWorkerGone(const CallbackInfo& info) {
+    return Boolean::New(info.Env(), current_worker == nullptr);
+  }
+
+  static void DeleteWorker(const CallbackInfo& info) {
+    if (current_worker == nullptr) {
+      Error::New(info.Env(), "No current worker").ThrowAsJavaScriptException();
+    }
+    delete current_worker;
+  }
+
+  ~PersistentTestWorker() {
+    current_worker = nullptr;
+  }
+
+protected:
+  void Execute() override {
+    if (!_succeed) {
+      SetError("test error");
+    }
+  }
+
+private:
+  PersistentTestWorker(Function cb,
+                       const char* resource_name,
+                       const Object& resource)
+      : AsyncWorker(cb, resource_name, resource) {}
+  bool _succeed;
+};
+
+PersistentTestWorker* PersistentTestWorker::current_worker = nullptr;
+
+Object InitPersistentAsyncWorker(Env env) {
+  Object exports = Object::New(env);
+  exports["doWork"] = Function::New(env, PersistentTestWorker::DoWork);
+  exports.DefineProperty(
+      PropertyDescriptor::Accessor("workerGone",
+                                   PersistentTestWorker::GetWorkerGone));
+  exports["deleteWorker"] =
+      Function::New(env, PersistentTestWorker::DeleteWorker);
+  return exports;
+}

test/asyncworker-persistent.js

@@ -0,0 +1,18 @@
+'use strict';
+const buildType = process.config.target_defaults.default_configuration;
+const assert = require('assert');
+const common = require('./common');
+
+test(require(`./build/${buildType}/binding.node`));
+test(require(`./build/${buildType}/binding_noexcept.node`));
+
+function test(binding) {
+  binding.persistentasyncworker.doWork(false, {}, function (e) {
+    setImmediate(() => {
+      assert.strictEqual(binding.persistentasyncworker.workerGone, false);
+      binding.persistentasyncworker.deleteWorker();
+      assert.strictEqual(binding.persistentasyncworker.workerGone, true);
+    });
+  }, 'test data');
+
+}

test/binding.cc

@@ -6,6 +6,7 @@ using namespace Napi;
 Object InitArrayBuffer(Env env);
 Object InitAsyncContext(Env env);
 Object InitAsyncWorker(Env env);
+Object InitPersistentAsyncWorker(Env env);
 Object InitBasicTypesArray(Env env);
 Object InitBasicTypesBoolean(Env env);
 Object InitBasicTypesNumber(Env env);
@@ -42,6 +43,7 @@ Object Init(Env env, Object exports) {
   exports.Set("arraybuffer", InitArrayBuffer(env));
   exports.Set("asynccontext", InitAsyncContext(env));
   exports.Set("asyncworker", InitAsyncWorker(env));
+  exports.Set("persistentasyncworker", InitPersistentAsyncWorker(env));
   exports.Set("basic_types_array", InitBasicTypesArray(env));
   exports.Set("basic_types_boolean", InitBasicTypesBoolean(env));
   exports.Set("basic_types_number", InitBasicTypesNumber(env));

test/binding.gyp

@@ -8,6 +8,7 @@
         'arraybuffer.cc',
         'asynccontext.cc',
         'asyncworker.cc',
+        'asyncworker-persistent.cc',
         'basic_types/array.cc',
         'basic_types/boolean.cc',
         'basic_types/number.cc',

test/index.js

@@ -11,6 +11,7 @@ let testModules = [
   'arraybuffer',
   'asynccontext',
   'asyncworker',
+  'asyncworker-persistent',
   'basic_types/array',
   'basic_types/boolean',
   'basic_types/number',