src: remove AsyncWorker move and complete tests

- Remove AsyncWorker move operators. These cannot work
  properly due to the linkage between the node-api
  and node-addon-api components
- Complete async worker test coverage for recv

PR-URL: #1266
Reviewed-By: Michael Dawson <[email protected]

Author: JckXia

napi-inl.h

@@ -4814,29 +4814,6 @@ inline void AsyncWorker::Destroy() {
   delete this;
 }
 
-inline AsyncWorker::AsyncWorker(AsyncWorker&& other) {
-  _env = other._env;
-  other._env = nullptr;
-  _work = other._work;
-  other._work = nullptr;
-  _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) {
-  _env = other._env;
-  other._env = nullptr;
-  _work = other._work;
-  other._work = nullptr;
-  _receiver = std::move(other._receiver);
-  _callback = std::move(other._callback);
-  _error = std::move(other._error);
-  _suppress_destruct = other._suppress_destruct;
-  return *this;
-}
-
 inline AsyncWorker::operator napi_async_work() const {
   return _work;
 }

napi.h

@@ -2439,9 +2439,6 @@ class AsyncWorker {
  public:
   virtual ~AsyncWorker();
 
-  // An async worker can be moved but cannot be copied.
-  AsyncWorker(AsyncWorker&& other);
-  AsyncWorker& operator=(AsyncWorker&& other);
   NAPI_DISALLOW_ASSIGN_COPY(AsyncWorker)
 
   operator napi_async_work() const;

test/async_worker.cc

@@ -7,6 +7,75 @@
 
 using namespace Napi;
 
+class TestWorkerWithUserDefRecv : public AsyncWorker {
+ public:
+  static void DoWork(const CallbackInfo& info) {
+    Object recv = info[0].As<Object>();
+    Function cb = info[1].As<Function>();
+
+    TestWorkerWithUserDefRecv* worker = new TestWorkerWithUserDefRecv(recv, cb);
+    worker->Queue();
+  }
+
+  static void DoWorkWithAsyncRes(const CallbackInfo& info) {
+    Object recv = info[0].As<Object>();
+    Function cb = info[1].As<Function>();
+    Object resource = info[2].As<Object>();
+
+    TestWorkerWithUserDefRecv* worker = nullptr;
+    if (resource == info.Env().Null()) {
+      worker = new TestWorkerWithUserDefRecv(recv, cb, "TestResource");
+    } else {
+      worker =
+          new TestWorkerWithUserDefRecv(recv, cb, "TestResource", resource);
+    }
+
+    worker->Queue();
+  }
+
+ protected:
+  void Execute() override {}
+
+ private:
+  TestWorkerWithUserDefRecv(const Object& recv, const Function& cb)
+      : AsyncWorker(recv, cb) {}
+  TestWorkerWithUserDefRecv(const Object& recv,
+                            const Function& cb,
+                            const char* resource_name)
+      : AsyncWorker(recv, cb, resource_name) {}
+  TestWorkerWithUserDefRecv(const Object& recv,
+                            const Function& cb,
+                            const char* resource_name,
+                            const Object& resource)
+      : AsyncWorker(recv, cb, resource_name, resource) {}
+};
+
+// Using default std::allocator impl, but assuming user can define their own
+// alloc/dealloc methods
+class CustomAllocWorker : public AsyncWorker {
+ public:
+  CustomAllocWorker(Function& cb) : AsyncWorker(cb){};
+  static void DoWork(const CallbackInfo& info) {
+    Function cb = info[0].As<Function>();
+    std::allocator<CustomAllocWorker> create_alloc;
+    CustomAllocWorker* newWorker = create_alloc.allocate(1);
+    create_alloc.construct(newWorker, cb);
+    newWorker->Queue();
+  }
+
+ protected:
+  void Execute() override {}
+  void Destroy() override {
+    assert(this->_secretVal == 24);
+    std::allocator<CustomAllocWorker> deallocer;
+    deallocer.destroy(this);
+    deallocer.deallocate(this, 1);
+  }
+
+ private:
+  int _secretVal = 24;
+};
+
 class TestWorker : public AsyncWorker {
  public:
   static void DoWork(const CallbackInfo& info) {
@@ -15,7 +84,13 @@ class TestWorker : public AsyncWorker {
     Function cb = info[2].As<Function>();
     Value data = info[3];
 
-    TestWorker* worker = new TestWorker(cb, "TestResource", resource);
+    TestWorker* worker = nullptr;
+    if (resource == info.Env().Null()) {
+      worker = new TestWorker(cb, "TestResource");
+    } else {
+      worker = new TestWorker(cb, "TestResource", resource);
+    }
+
     worker->Receiver().Set("data", data);
     worker->_succeed = succeed;
     worker->Queue();
@@ -31,6 +106,8 @@ class TestWorker : public AsyncWorker {
  private:
   TestWorker(Function cb, const char* resource_name, const Object& resource)
       : AsyncWorker(cb, resource_name, resource) {}
+  TestWorker(Function cb, const char* resource_name)
+      : AsyncWorker(cb, resource_name) {}
   bool _succeed;
 };
 
@@ -72,12 +149,25 @@ class TestWorkerWithResult : public AsyncWorker {
 class TestWorkerNoCallback : public AsyncWorker {
  public:
   static Value DoWork(const CallbackInfo& info) {
+    bool succeed = info[0].As<Boolean>();
+
+    TestWorkerNoCallback* worker = new TestWorkerNoCallback(info.Env());
+    worker->_succeed = succeed;
+    worker->Queue();
+    return worker->_deferred.Promise();
+  }
+
+  static Value DoWorkWithAsyncRes(const CallbackInfo& info) {
     napi_env env = info.Env();
     bool succeed = info[0].As<Boolean>();
     Object resource = info[1].As<Object>();
 
-    TestWorkerNoCallback* worker =
-        new TestWorkerNoCallback(env, "TestResource", resource);
+    TestWorkerNoCallback* worker = nullptr;
+    if (resource == info.Env().Null()) {
+      worker = new TestWorkerNoCallback(env, "TestResource");
+    } else {
+      worker = new TestWorkerNoCallback(env, "TestResource", resource);
+    }
     worker->_succeed = succeed;
     worker->Queue();
     return worker->_deferred.Promise();
@@ -91,6 +181,13 @@ class TestWorkerNoCallback : public AsyncWorker {
   }
 
  private:
+  TestWorkerNoCallback(Napi::Env env)
+      : AsyncWorker(env), _deferred(Napi::Promise::Deferred::New(env)) {}
+
+  TestWorkerNoCallback(napi_env env, const char* resource_name)
+      : AsyncWorker(env, resource_name),
+        _deferred(Napi::Promise::Deferred::New(env)) {}
+
   TestWorkerNoCallback(napi_env env,
                        const char* resource_name,
                        const Object& resource)
@@ -222,7 +319,12 @@ class CancelWorker : public AsyncWorker {
 
 Object InitAsyncWorker(Env env) {
   Object exports = Object::New(env);
+  exports["doWorkRecv"] = Function::New(env, TestWorkerWithUserDefRecv::DoWork);
+  exports["doWithRecvAsyncRes"] =
+      Function::New(env, TestWorkerWithUserDefRecv::DoWorkWithAsyncRes);
   exports["doWork"] = Function::New(env, TestWorker::DoWork);
+  exports["doWorkAsyncResNoCallback"] =
+      Function::New(env, TestWorkerNoCallback::DoWorkWithAsyncRes);
   exports["doWorkNoCallback"] =
       Function::New(env, TestWorkerNoCallback::DoWork);
   exports["doWorkWithResult"] =
@@ -231,5 +333,7 @@ Object InitAsyncWorker(Env env) {
 
   exports["expectCancelToFail"] =
       Function::New(env, FailCancelWorker::DoCancel);
+  exports["expectCustomAllocWorkerToDealloc"] =
+      Function::New(env, CustomAllocWorker::DoWork);
   return exports;
 }

test/async_worker.js

@@ -76,8 +76,20 @@ async function test (binding) {
   assert.equal(taskFailed, true, 'We expect task cancellation to fail');
 
   if (!checkAsyncHooks()) {
+    binding.asyncworker.expectCustomAllocWorkerToDealloc(() => {});
+
+    await new Promise((resolve) => {
+      const obj = { data: 'test data' };
+      binding.asyncworker.doWorkRecv(obj, function (e) {
+        assert.strictEqual(typeof e, 'undefined');
+        assert.strictEqual(typeof this, 'object');
+        assert.strictEqual(this.data, 'test data');
+        resolve();
+      });
+    });
+
     await new Promise((resolve) => {
-      binding.asyncworker.doWork(true, {}, function (e) {
+      binding.asyncworker.doWork(true, null, function (e) {
         assert.strictEqual(typeof e, 'undefined');
         assert.strictEqual(typeof this, 'object');
         assert.strictEqual(this.data, 'test data');
@@ -109,6 +121,62 @@ async function test (binding) {
     return;
   }
 
+  {
+    const hooks = installAsyncHooksForTest();
+    const triggerAsyncId = asyncHooks.executionAsyncId();
+    await new Promise((resolve) => {
+      const recvObj = { data: 'test data' };
+      binding.asyncworker.doWithRecvAsyncRes(recvObj, function (e) {
+        assert.strictEqual(typeof e, 'undefined');
+        assert.strictEqual(typeof this, 'object');
+        assert.strictEqual(this.data, 'test data');
+        resolve();
+      }, { foo: 'fooBar' });
+    });
+
+    await hooks.then(actual => {
+      assert.deepStrictEqual(actual, [
+        {
+          eventName: 'init',
+          type: 'TestResource',
+          triggerAsyncId: triggerAsyncId,
+          resource: { foo: 'fooBar' }
+        },
+        { eventName: 'before' },
+        { eventName: 'after' },
+        { eventName: 'destroy' }
+      ]);
+    }).catch(common.mustNotCall());
+  }
+
+  {
+    const hooks = installAsyncHooksForTest();
+    const triggerAsyncId = asyncHooks.executionAsyncId();
+    await new Promise((resolve) => {
+      const recvObj = { data: 'test data' };
+      binding.asyncworker.doWithRecvAsyncRes(recvObj, function (e) {
+        assert.strictEqual(typeof e, 'undefined');
+        assert.strictEqual(typeof this, 'object');
+        assert.strictEqual(this.data, 'test data');
+        resolve();
+      }, null);
+    });
+
+    await hooks.then(actual => {
+      assert.deepStrictEqual(actual, [
+        {
+          eventName: 'init',
+          type: 'TestResource',
+          triggerAsyncId: triggerAsyncId,
+          resource: { }
+        },
+        { eventName: 'before' },
+        { eventName: 'after' },
+        { eventName: 'destroy' }
+      ]);
+    }).catch(common.mustNotCall());
+  }
+
   {
     const hooks = installAsyncHooksForTest();
     const triggerAsyncId = asyncHooks.executionAsyncId();

test/async_worker_nocallback.js

@@ -5,9 +5,15 @@ const common = require('./common');
 module.exports = common.runTest(test);
 
 async function test (binding) {
-  await binding.asyncworker.doWorkNoCallback(true, {})
+  await binding.asyncworker.doWorkAsyncResNoCallback(true, {})
     .then(common.mustCall()).catch(common.mustNotCall());
 
-  await binding.asyncworker.doWorkNoCallback(false, {})
+  await binding.asyncworker.doWorkAsyncResNoCallback(false, {})
+    .then(common.mustNotCall()).catch(common.mustCall());
+
+  await binding.asyncworker.doWorkNoCallback(false)
+    .then(common.mustNotCall()).catch(common.mustCall());
+
+  await binding.asyncworker.doWorkNoCallback(true)
     .then(common.mustNotCall()).catch(common.mustCall());
 }