Chaining two AsyncWorker calls

[bsrdjan]

One n-api addon accepting two JavaScript parameters, A and B, should internally call two AsyncWorker methods, one after the other and return the 2nd method result by callback call.

The 1st AsyncWorker method shall be invoked with JS parameter A.

The 2nd method shall be invoked with 1st method result and JS parameter B.

I tried to pass both A and B parameters (and callback arg) to 1st AsyncWorker method, keep B as 1st AsyncWorker private variable and use it in OnOK, to invoke the 2nd AsyncWorker method.

The B parameter seem to be corrupted in Execute and in OnOK methods of the 1st AccessWorker. Access causes the segfault.

I tested by modifying async_pi_estimate original example.

Uncommenting the printf statement in Execute leads to segfault.

async.cc

#include <napi.h>
#include "pi_est.h"  // NOLINT(build/include)
#include "async.h"  // NOLINT(build/include)

class PiWorker : public Napi::AsyncWorker {
 public:
  PiWorker(Napi::Function& callback, int points, Napi::String str1)
    : Napi::AsyncWorker(callback), points(points), str1(str1), estimate(0) {
      printf ("\nconstructor:: %d\n", str1.IsString());
    }
  ~PiWorker() {}

  // Executed inside the worker-thread.
  // It is not safe to access JS engine data structure
  // here, so everything we need for input and output
  // should go on `this`.
  void Execute () {
    estimate = Estimate(points);
    //printf ("\nexecute:: %d\n", str1.IsString());
  }

  // Executed when the async work is complete
  // this function will be run inside the main event loop
  // so it is safe to use JS engine data again
  void OnOK() {
    //if (!str1.IsString()) printf ("\nok:: %d\n", str1.IsString());
    Napi::HandleScope scope(Env());
    Callback().Call({Env().Global(), Napi::Number::New(Env(), estimate)});
  }

 private:
  int points;
  Napi::String str1;
  double estimate;
};

// Asynchronous access to the `Estimate()` function
Napi::Value CalculateAsync(const Napi::CallbackInfo& info) {
  int points = info[0].As<Napi::Number>().Uint32Value();
  Napi::Function callback = info[1].As<Napi::Function>();
  Napi::String str1 = info[2].As<Napi::String>(); // segfault in Execute
  //Napi::String str1 = Napi::String::New(info.Env(), "Test"); // no segfault
  PiWorker* piWorker = new PiWorker(callback, points, str1);
  piWorker->Queue();
  return info.Env().Undefined();
}

addon.js

var addon = require('bindings')('addon');
var calculations = process.argv[3] || 100000000;

function printResult(type, pi, ms) {
	console.log(type, 'method:');
	console.log('\tπ ≈ ' + pi + ' (' + Math.abs(pi - Math.PI) + ' away from actual)');
	console.log('\tTook ' + ms + 'ms');
	console.log();
}

function runSync(str) {
	var start = Date.now();
	// Estimate() will execute in the current thread,
	// the next line won't return until it is finished
	var result = addon.calculateSync(calculations, str);
	printResult('Sync', result, Date.now() - start);
}

function runAsync(str) {
	// how many batches should we split the work in to?
	var batches = process.argv[4] || 16;
	var ended = 0;
	var total = 0;
	var start = Date.now();

	function done(err, result) {
		total += result;

		// have all the batches finished executing?
		if (++ended === batches) {
			printResult('Async', total / batches, Date.now() - start);
		}
	}

	// for each batch of work, request an async Estimate() for
	// a portion of the total number of calculations
	for (var i = 0; i < batches; i++) {
		addon.calculateAsync(calculations / batches, done, str);
	}
}

//runSync(process.argv[2]);
runAsync(process.argv[2]);

Is it how chained AsyncWorker calls could/should work or what would be the correct approach? Any working example available?

The issue happens only with parameters from command line (eg. node addon test), see 4th and 5th line from the bottom.

[gabrielschulhof]

@bsrdjan you must not make N-API-related calls from the Execute() callback, because the body of that callback runs on a thread other than the Node.js main thread, and the JavaScript engine is not thread-safe.

[gabrielschulhof]

If you move the printf() statement from Execute() to OnOK() does it still segfault?

[bsrdjan]

If moved to OnOK, the console output is:

Error: Invalid argument
    at Error (native)

[bsrdjan]

Thanks @gabrielschulhof for the clarification about Execute().

Any ideas how two consecutive async calls for the above mentioned scenario could be implemented?

[gabrielschulhof]

@bsrdjan I modified the example to launch another asynchronous pi computation when it's finished:

diff --git a/async_pi_estimate/node-addon-api/addon.js b/async_pi_estimate/node-addon-api/addon.js
index d52b3a3..78e02c1 100755
--- a/async_pi_estimate/node-addon-api/addon.js
+++ b/async_pi_estimate/node-addon-api/addon.js
@@ -30,6 +30,9 @@ function runAsync() {
     // have all the batches finished executing?
     if (++ended === batches) {
       printResult('Async', total / batches, Date.now() - start);
+      // done() will be called again, so reset counters.
+      ended = 0;
+      start = Date.now();
     }
   }
 
diff --git a/async_pi_estimate/node-addon-api/async.cc b/async_pi_estimate/node-addon-api/async.cc
old mode 100755
new mode 100644
index a23a2f5..f448e64
--- a/async_pi_estimate/node-addon-api/async.cc
+++ b/async_pi_estimate/node-addon-api/async.cc
@@ -4,9 +4,10 @@
 
 class PiWorker : public Napi::AsyncWorker {
  public:
-  PiWorker(Napi::Function& callback, int points)
-    : Napi::AsyncWorker(callback), points(points), estimate(0) {}
-  ~PiWorker() {}
+  PiWorker(Napi::Function& callback, int points, bool run_again = true)
+    : Napi::AsyncWorker(callback), points(points), estimate(0),
+      callback(Napi::Persistent(callback)), run_again(run_again) {
+  }
 
   // Executed inside the worker-thread.
   // It is not safe to access JS engine data structure
@@ -22,11 +23,18 @@ class PiWorker : public Napi::AsyncWorker {
   void OnOK() {
     Napi::HandleScope scope(Env());
     Callback().Call({Env().Undefined(), Napi::Number::New(Env(), estimate)});
+    if (run_again) {
+      auto value = callback.Value();
+      (new PiWorker(value, points, false))->Queue();
+      callback.Reset();
+    }
   }
 
  private:
   int points;
   double estimate;
+  Napi::FunctionReference callback;
+  bool run_again;
 };
 
 // Asynchronous access to the `Estimate()` function

With this change, the PiWorker class not only passes callback to the parent class constructor, but also stores it locally in a FunctionReference created with Napi::Persistent(). In OnOK() it then retrieves the Napi::Function from the Napi::FunctionReference and passes it to another copy of PiWorker which it then queues for eventual execution. It then resets the Napi::FunctionReference because, having passed the JavaScript function to the second and last copy of itself, it no longer needs to hold on to it.

[bsrdjan]

Thank you very much for the working example. Reading the OnOK method comment:

Executed when the async work is complete this function will be run inside the main event loop so it is safe to use JS engine data again

I tried to pass the Napi::Array to AsyncWorker and reuse it in OnOK method, as below:

class PiWorker : public Napi::AsyncWorker
{
public:
  PiWorker(Napi::Function &callback, int points, Napi::Array arr)
      : Napi::AsyncWorker(callback), points(points), estimate(0),
        callback(Napi::Persistent(callback)), arr(arr)
  {
    printf("C arr: %d\n", arr.Length());
  };

  // Executed inside the worker-thread.
  // It is not safe to access JS engine data structure
  // here, so everything we need for input and output
  // should go on `this`.
  void Execute()
  {
    estimate = Estimate(points);
  }

  // Executed when the async work is complete
  // this function will be run inside the main event loop
  // so it is safe to use JS engine data again
  void OnOK()
  {
    Napi::HandleScope scope(Env());
    printf("OK arr: %d\n", arr.Length());
    Callback().Call({Env().Undefined(), Napi::Number::New(Env(), estimate)});
  }

private:
  int points;
  double estimate;
  Napi::FunctionReference callback;
  Napi::Array arr;
};

// Asynchronous access to the `Estimate()` function
Napi::Value CalculateAsync(const Napi::CallbackInfo &info)
{
  int points = info[0].As<Napi::Number>().Uint32Value();
  Napi::Function callback = info[1].As<Napi::Function>();
  Napi::Array arr = Napi::Array::New(info.Env(), 5);
  printf("arr: %d\n", arr.Length());
  PiWorker *piWorker = new PiWorker(callback, points, arr);
  piWorker->Queue();
  return info.Env().Undefined();
}

The length of 5 is correctly printed in constructor and following error comes up from OnOK printf statement:

Error: An array was expected
    at Error (native)

Shall AsyncWorker safely keep the array for the reuse in OnOK or what else could be possibly wrong here?

[gabrielschulhof]

@bsrdjan values of the form Napi::Array, Napi::Number, Napi::String, etc. can only be used inside the function which receives them. They cannot be stored, because after the function that receives them returns, they are no longer valid.

In order to store a value during the constructor so you might retrieve it during OnOK() you need to store it in a Napi::FunctionReference or a Napi::ObjectReference in the private part of the class – as I did with callback. After you retrieve the reference in OnOK() you need to reset it so it holds nothing, otherwise it'll keep holding on to whatever you put in it, thus preventing the JavaScript engine from garbage-collecting that value.

So, you should declare

Napi::ObjectReference arr;

in the private section of your class and you should construct it with arr(Napi::Persistent(arr)). Then, after retrieving and using the array in OnOK() you should arr.Reset() it to tell the garbage collector that it may collect the array if nobody else is using it.

[bsrdjan]

Thanks, the lesson #2 helped :)

[p3x-robot]

Sorry, but can I ask? So if the OnOk is already on the main thread, but I have a pointer array, and I have to use a Napi::Array, but I cannot create the Napi::Array in the Execute method, only on the OnOk method. But I have to transform from the the pointer array to Napi::Array and I can only use on the main thread, so how can I do this transformation without the loss of penalty? (3000 transformation with a for, and it only possible in the OnOk method, but looks like a big penalty).