AsyncProgressWorker::OnProgress is invoked without arg sometimes #821
Comments
|
@zbjornson can you share the code? Since you can't recreate with the sample, its probably something in the code that you've not shared in the issue. |
|
@mhdawson here's a repro minimally modified from the example (only #include <chrono>
#include <thread>
#include <string>
using namespace Napi;
struct ProgressInfo {
float progress;
const char* status;
};
class EchoWorker : public AsyncProgressWorker<ProgressInfo> {
public:
EchoWorker(Function& callback, std::string& echo)
: AsyncProgressWorker(callback), echo(echo) {}
~EchoWorker() {}
void Execute(const ExecutionProgress& progress) {
for (uint32_t i = 0; i < 500; ++i) {
for (uint32_t j = 0; j < 5; ++j) {
ProgressInfo pi{i * j, "running"};
printf("sending %f\n", 1.0f * i * j);
progress.Send(&pi, 1);
}
std::this_thread::sleep_for(std::chrono::milliseconds(2));
}
}
void OnOK() {
HandleScope scope(Env());
Callback().Call({Env().Null(), String::New(Env(), echo)});
}
void OnProgress(const ProgressInfo* data, size_t count) {
if (count == 1) {
printf("progress %f\n", data->progress);
} else {
printf("count is 0!\n"); abort();
}
HandleScope scope(Env());
Callback().Call({Env().Null(), Env().Null(), Number::New(Env(), data->progress)});
}
private:
std::string echo;
};
Value Echo(const CallbackInfo& info) {
// We need to validate the arguments here
Function cb = info[1].As<Function>();
std::string in = info[0].As<String>();
EchoWorker* wk = new EchoWorker(cb, in);
wk->Queue();
return info.Env().Undefined();
}
Napi::Object Init(Napi::Env env, Napi::Object exports) {
exports.Set(Napi::String::New(env, "Echo"), Napi::Function::New(env, Echo));
return exports;
}mymodule.Echo("a", console.log);This prints something like: |
|
Can you give me all the files for the cut down example needed to build/run? Possibly not relevant, I'm wondering if there is something going on with struct ProgressInfo {
float progress;
const char* status;
};
|
|
Sure, full repo here: https://gist.github.com/zbjornson/5edc22bb7381e4c2a1f00aee44abe4b7 (or I pared it down further so it's just |
|
@zbjornson thanks for your detailed re-produce and reporting here! I investigated the re-produce and found out the issue may be related to the timing of SendProgress been called during two async callback of thread-safe function:
As far as I can tell, the AsyncProgressWorker guarantees at least once of the progress callback. However, in the issue, the callback was called with unexpected data (data with I'll file a fix. |
|
Thank you! |
Fixes: nodejs/node-addon-api#821 PR-URL: nodejs/node-addon-api#853 Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
Fixes: nodejs/node-addon-api#821 PR-URL: nodejs/node-addon-api#853 Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
Fixes: nodejs/node-addon-api#821 PR-URL: nodejs/node-addon-api#853 Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
Fixes: nodejs/node-addon-api#821 PR-URL: nodejs/node-addon-api#853 Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
I've got a basic AsyncProgressWorker that's very similar to the example. I'm invoking
progress.Send(thing, 1)a few hundred times per second, and seeingOnProgressinvoked sometimes with(info=nullptr, count=0):It happens with less frequent invocations also, but far less frequently.
That's easy enough to guard for, but is that expected? I don't see it mentioned in the docs.
Edit 13-Oct I can't find a way to reproduce this by modifying the example. In my real scenario, the async work is CUDA code. Not sure if that's different in some critical way from sleeping the thread. Here's a pair of strace captures from when count=0 (top) and count=1 (bottom):
The text was updated successfully, but these errors were encountered: