Skip to content

Commit

Permalink
src: remove AsyncWorker move and complete tests
Browse files Browse the repository at this point in the history 
- 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 <midawson@redhat.com
  • Loading branch information
@JckXia @mhdawson
JckXia authored and mhdawson committed Jan 30, 2023
1 parent ff96948 commit e272619
Show file tree
Hide file tree
Showing 5 changed files with 184 additions and 32 deletions.
23 changes: 0 additions & 23 deletions napi-inl.h
View file
Expand Up @@ -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;
}
Expand Down
3 changes: 0 additions & 3 deletions napi.h
View file
Expand Up @@ -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;
Expand Down
110 changes: 107 additions & 3 deletions test/async_worker.cc
View file
Expand Up @@ -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) {
Expand All @@ -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();
Expand All @@ -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;
};

Expand Down Expand Up @@ -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();
Expand All @@ -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)
Expand Down Expand Up @@ -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"] =
Expand All @@ -231,5 +333,7 @@ Object InitAsyncWorker(Env env) {

exports["expectCancelToFail"] =
Function::New(env, FailCancelWorker::DoCancel);
exports["expectCustomAllocWorkerToDealloc"] =
Function::New(env, CustomAllocWorker::DoWork);
return exports;
}
70 changes: 69 additions & 1 deletion test/async_worker.js
View file
Expand Up @@ -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');
Expand Down Expand Up @@ -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();
Expand Down
10 changes: 8 additions & 2 deletions test/async_worker_nocallback.js
View file
Expand Up @@ -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());
}

0 comments on commit e272619

Please sign in to comment.