Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add coverage for napi_cancel_async_work #12575

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion test/addons-napi/test_async/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@ const common = require('../../common');
const assert = require('assert');
const test_async = require(`./build/${common.buildType}/test_async`);

test_async(5, common.mustCall(function(err, val) {
test_async.Test(5, common.mustCall(function(err, val) {
assert.strictEqual(err, null);
assert.strictEqual(val, 10);
process.nextTick(common.mustCall(function() {}));
}));

const cancelSuceeded = function() {};
test_async.TestCancel(common.mustCall(cancelSuceeded));
80 changes: 75 additions & 5 deletions test/addons-napi/test_async/test_async.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
#include <unistd.h>
#endif

// this needs to be greater than the thread pool size
#define MAX_CANCEL_THREADS 6

typedef struct {
int32_t _input;
int32_t _output;
Expand All @@ -15,6 +18,7 @@ typedef struct {
} carrier;

carrier the_carrier;
carrier async_carrier[MAX_CANCEL_THREADS];

struct AutoHandleScope {
explicit AutoHandleScope(napi_env env)
Expand Down Expand Up @@ -111,12 +115,78 @@ napi_value Test(napi_env env, napi_callback_info info) {
return nullptr;
}

void BusyCancelComplete(napi_env env, napi_status status, void* data) {
AutoHandleScope scope(env);
carrier* c = static_cast<carrier*>(data);
NAPI_CALL_RETURN_VOID(env, napi_delete_async_work(env, c->_request));
}

void CancelComplete(napi_env env, napi_status status, void* data) {
AutoHandleScope scope(env);
carrier* c = static_cast<carrier*>(data);

if (status == napi_cancelled) {
// ok we got the status we expected so make the callback to
// indicate the cancel succeeded.
napi_value callback;
NAPI_CALL_RETURN_VOID(env,
napi_get_reference_value(env, c->_callback, &callback));
napi_value global;
NAPI_CALL_RETURN_VOID(env, napi_get_global(env, &global));
napi_value result;
NAPI_CALL_RETURN_VOID(env,
napi_call_function(env, global, callback, 0, nullptr, &result));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

napi_make_callback?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I had re-used code from the other test in the file, but in this case it should be napi_make_callback

}

NAPI_CALL_RETURN_VOID(env, napi_delete_async_work(env, c->_request));
NAPI_CALL_RETURN_VOID(env, napi_delete_reference(env, c->_callback));
}

void CancelExecute(napi_env env, void* data) {
#if defined _WIN32
Sleep(2000);
#else
sleep(2);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this duration guessed? I would assume we could get away with something shorter…

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I just chose a time that I thought would be long enough. Its a trade off between how long the test runs and potentially flaky failures.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On my machine 1s is fine as well, but I'd lean towards leaving at 2s as I'd rather avoid risking flaky failures.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @nodejs/testing has a general guideline that tests should not take longer than 1 second to run (which makes sense, we have thousands of tests…), that’s why I’m asking

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok changed to 1s

#endif
}

napi_value TestCancel(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value argv[1];
napi_value _this;
void* data;

// make sure the work we are going to cancel will not be
// able to start by using all the threads in the pool
for (int i = 1; i < MAX_CANCEL_THREADS; i++) {
NAPI_CALL(env, napi_create_async_work(env, CancelExecute,
BusyCancelComplete, &async_carrier[i], &async_carrier[i]._request));
NAPI_CALL(env, napi_queue_async_work(env, async_carrier[i]._request));
}

// now queue the work we are going to cancel and then cancel it.
// cancel will fail if the work has already started, but
// we have prevented it from starting by consuming all of the
// workers above.
NAPI_CALL(env,
napi_get_cb_info(env, info, &argc, argv, &_this, &data));
NAPI_CALL(env, napi_create_async_work(env, CancelExecute,
CancelComplete, &async_carrier[0], &async_carrier[0]._request));
NAPI_CALL(env,
napi_create_reference(env, argv[0], 1, &async_carrier[0]._callback));
NAPI_CALL(env, napi_queue_async_work(env, async_carrier[0]._request));
NAPI_CALL(env, napi_cancel_async_work(env, async_carrier[0]._request));
return nullptr;
}

void Init(napi_env env, napi_value exports, napi_value module, void* priv) {
napi_value test;
NAPI_CALL_RETURN_VOID(env,
napi_create_function(env, "Test", Test, nullptr, &test));
NAPI_CALL_RETURN_VOID(env,
napi_set_named_property(env, module, "exports", test));
napi_property_descriptor properties[] = {
DECLARE_NAPI_PROPERTY("Test", Test),
DECLARE_NAPI_PROPERTY("TestCancel", TestCancel),
};

NAPI_CALL_RETURN_VOID(env, napi_define_properties(
env, exports, sizeof(properties) / sizeof(*properties), properties));
}

NAPI_MODULE(addon, Init)