diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e85a66c3f..193f09384 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,6 +4,7 @@ on: [push, pull_request] jobs: test: + timeout-minutes: 30 strategy: matrix: node-version: diff --git a/napi-inl.h b/napi-inl.h index 5a345df53..4e8e8ef11 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -5390,6 +5390,17 @@ inline void AsyncProgressWorker::OnWorkProgress(void*) { this->_asyncsize = 0; } + /** + * The callback of ThreadSafeFunction is not been invoked immediately on the + * callback of uv_async_t (uv io poll), rather the callback of TSFN is + * invoked on the right next uv idle callback. There are chances that during + * the deferring the signal of uv_async_t is been sent again, i.e. potential + * not coalesced two calls of the TSFN callback. + */ + if (data == nullptr) { + return; + } + this->OnProgress(data, size); delete[] data; } diff --git a/test/asyncprogressworker.cc b/test/asyncprogressworker.cc index 8ac062fcb..1705124ad 100644 --- a/test/asyncprogressworker.cc +++ b/test/asyncprogressworker.cc @@ -61,11 +61,69 @@ class TestWorker : public AsyncProgressWorker { FunctionReference _progress; }; +class MalignWorker : public AsyncProgressWorker { + public: + static void DoWork(const CallbackInfo& info) { + Function cb = info[0].As(); + Function progress = info[1].As(); + + MalignWorker* worker = + new MalignWorker(cb, progress, "TestResource", Object::New(info.Env())); + worker->Queue(); + } + + protected: + void Execute(const ExecutionProgress& progress) override { + std::unique_lock lock(_cvm); + // Testing a nullptr send is acceptable. + progress.Send(nullptr, 0); + _cv.wait(lock); + // Testing busy looping on send doesn't trigger unexpected empty data + // OnProgress call. + for (size_t i = 0; i < 1000000; i++) { + ProgressData data{0}; + progress.Send(&data, 1); + } + } + + void OnProgress(const ProgressData* /* data */, size_t count) override { + Napi::Env env = Env(); + _test_case_count++; + bool error = false; + Napi::String reason = Napi::String::New(env, "No error"); + if (_test_case_count == 1 && count != 0) { + error = true; + reason = Napi::String::New(env, "expect 0 count of data on 1st call"); + } + if (_test_case_count > 1 && count != 1) { + error = true; + reason = Napi::String::New(env, "expect 1 count of data on non-1st call"); + } + _progress.MakeCallback(Receiver().Value(), + {Napi::Boolean::New(env, error), reason}); + _cv.notify_one(); + } + + private: + MalignWorker(Function cb, + Function progress, + const char* resource_name, + const Object& resource) + : AsyncProgressWorker(cb, resource_name, resource) { + _progress.Reset(progress, 1); + } + + size_t _test_case_count = 0; + std::condition_variable _cv; + std::mutex _cvm; + FunctionReference _progress; +}; } Object InitAsyncProgressWorker(Env env) { Object exports = Object::New(env); exports["doWork"] = Function::New(env, TestWorker::DoWork); + exports["doMalignTest"] = Function::New(env, MalignWorker::DoWork); return exports; } diff --git a/test/asyncprogressworker.js b/test/asyncprogressworker.js index b2896a6ca..285fd2a91 100644 --- a/test/asyncprogressworker.js +++ b/test/asyncprogressworker.js @@ -9,6 +9,7 @@ module.exports = test(require(`./build/${buildType}/binding.node`)) async function test({ asyncprogressworker }) { await success(asyncprogressworker); await fail(asyncprogressworker); + await malignTest(asyncprogressworker); } function success(binding) { @@ -43,3 +44,19 @@ function fail(binding) { ); }); } + +function malignTest(binding) { + return new Promise((resolve, reject) => { + binding.doMalignTest( + common.mustCall((err) => { + if (err) { + return reject(err); + } + resolve(); + }), + common.mustCallAtLeast((error, reason) => { + assert(!error, reason); + }, 1) + ); + }); +} diff --git a/test/common/index.js b/test/common/index.js index 57bc525ef..54139bb2d 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -33,6 +33,9 @@ function runCallChecks(exitCode) { exports.mustCall = function(fn, exact) { return _mustCallInner(fn, exact, 'exact'); }; +exports.mustCallAtLeast = function(fn, minimum) { + return _mustCallInner(fn, minimum, 'minimum'); +}; function _mustCallInner(fn, criteria, field) { if (typeof fn === 'number') {