Skip to content

Commit

Permalink
Additional changes from review
Browse files Browse the repository at this point in the history
  • Loading branch information
KevinEady committed Oct 10, 2020
1 parent 59f27da commit b54f5eb
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 61 deletions.
9 changes: 5 additions & 4 deletions doc/threadsafe.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,11 @@ construct a no-op `Function` **or** to target N-API 5 and "construct" a
with just a switch of the `NAPI_VERSION` compile-time constant.

The removal of the dynamic call functionality has the following implications:
- The API does _not_ act as a "broker" compared to the non-`Ex`. Once Node.js
finalizes the thread-safe function, the `CallJs` callback will execute with an
empty `Napi::Env` for any remaining items on the queue. This provides the
ability to handle any necessary cleanup of the item's data.
- The API does _not_ act as a "broker" compared to the
`Napi::ThreadSafeFunction`. Once Node.js finalizes the thread-safe function,
the `CallJs` callback will execute with an empty `Napi::Env` for any remaining
items on the queue. This provides the ability to handle any necessary cleanup
of the item's data.
- The callback _does_ receive the context as a parameter, so a call to
`GetContext()` is _not_ necessary. This context type is specified as the
**first template argument** specified to `::New`, ensuring type safety.
Expand Down
97 changes: 44 additions & 53 deletions doc/threadsafe_function_ex.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ Returns one of:
Indicates that an existing thread will stop making use of the thread-safe
function. A thread should call this API when it stops making use of this
thread-safe function. Using any thread-safe APIs after having called this API
has undefined results in the current thread, as the thread-safe function may have been destroyed.
has undefined results in the current thread, as the thread-safe function may
have been destroyed.

```cpp
napi_status Napi::ThreadSafeFunctionEx<ContextType, DataType, Callback>::Release()
Expand Down Expand Up @@ -176,7 +177,8 @@ Returns one of:
- `napi_ok`: `data` was successfully added to the queue.
- `napi_queue_full`: The queue was full when trying to call in a non-blocking
method.
- `napi_closing`: The thread-safe function is aborted and no further calls can be made.
- `napi_closing`: The thread-safe function is aborted and no further calls can
be made.
- `napi_invalid_arg`: The thread-safe function is closed.
- `napi_generic_failure`: A generic error occurred when attemping to add to the
queue.
Expand All @@ -186,110 +188,99 @@ Returns one of:

```cpp
#include <chrono>
#include <thread>
#include <napi.h>
#include <thread>

using namespace Napi;

using Context = Reference<Value>;
using DataType = int;
void CallJs( Napi::Env env, Function callback, Context* context, DataType* data );
void CallJs(Napi::Env env, Function callback, Context *context, DataType *data);
using TSFN = ThreadSafeFunctionEx<Context, DataType, CallJs>;
using FinalizerDataType = void;

std::thread nativeThread;
TSFN tsfn;

Value Start( const CallbackInfo& info )
{
Value Start(const CallbackInfo &info) {
Napi::Env env = info.Env();

if ( info.Length() < 2 )
{
throw TypeError::New( env, "Expected two arguments" );
}
else if ( !info[0].IsFunction() )
{
throw TypeError::New( env, "Expected first arg to be function" );
}
else if ( !info[1].IsNumber() )
{
throw TypeError::New( env, "Expected second arg to be number" );
if (info.Length() < 2) {
throw TypeError::New(env, "Expected two arguments");
} else if (!info[0].IsFunction()) {
throw TypeError::New(env, "Expected first arg to be function");
} else if (!info[1].IsNumber()) {
throw TypeError::New(env, "Expected second arg to be number");
}

int count = info[1].As<Number>().Int32Value();

// Create a new context set to the the receiver (ie, `this`) of the function
// call
Context* context = new Reference<Value>( Persistent( info.This() ) );
Context *context = new Reference<Value>(Persistent(info.This()));

// Create a ThreadSafeFunction
tsfn = TSFN::New( env,
info[0].As<Function>(), // JavaScript function called asynchronously
"Resource Name", // Name
0, // Unlimited queue
1, // Only one thread will use this initially
context,
[]( Napi::Env, FinalizerDataType*,
Context* ctx ) { // Finalizer used to clean threads up
nativeThread.join();
delete ctx;
} );
tsfn = TSFN::New(
env,
info[0].As<Function>(), // JavaScript function called asynchronously
"Resource Name", // Name
0, // Unlimited queue
1, // Only one thread will use this initially
context,
[](Napi::Env, FinalizerDataType *,
Context *ctx) { // Finalizer used to clean threads up
nativeThread.join();
delete ctx;
});

// Create a native thread
nativeThread = std::thread( [count] {
for ( int i = 0; i < count; i++ )
{
nativeThread = std::thread([count] {
for (int i = 0; i < count; i++) {
// Create new data
int* value = new int( clock() );
int *value = new int(clock());

// Perform a blocking call
napi_status status = tsfn.BlockingCall( value );
if ( status != napi_ok )
{
napi_status status = tsfn.BlockingCall(value);
if (status != napi_ok) {
// Handle error
break;
}

std::this_thread::sleep_for( std::chrono::seconds( 1 ) );
std::this_thread::sleep_for(std::chrono::seconds(1));
}

// Release the thread-safe function
tsfn.Release();
} );
});

return Boolean::New( env, true );
return Boolean::New(env, true);
}

// Transform native data into JS data, passing it to the provided
// `callback` -- the TSFN's JavaScript function.
void CallJs( Napi::Env env, Function callback, Context* context, DataType* data )
{
void CallJs(Napi::Env env, Function callback, Context *context,
DataType *data) {
// Is the JavaScript environment still available to call into, eg. the TSFN is
// not aborted
if ( env != nullptr )
{
if (env != nullptr) {
// On N-API 5+, the `callback` parameter is optional; however, this example
// does ensure a callback is provided.
if ( callback != nullptr )
{
callback.Call( context->Value(), {Number::New( env, *data )} );
if (callback != nullptr) {
callback.Call(context->Value(), {Number::New(env, *data)});
}
}
if ( data != nullptr )
{
if (data != nullptr) {
// We're finished with the data.
delete data;
}
}

Napi::Object Init( Napi::Env env, Object exports )
{
exports.Set( "start", Function::New( env, Start ) );
Napi::Object Init(Napi::Env env, Object exports) {
exports.Set("start", Function::New(env, Start));
return exports;
}

NODE_API_MODULE( clock, Init )
NODE_API_MODULE(clock, Init)
```
The above code can be used from JavaScript as follows:
Expand All @@ -304,7 +295,7 @@ start.call(new Date(), function (clock) {
```

When executed, the output will show the value of `clock()` five times at one
second intervals, prefixed with the TSFN's context -- `start`'s receiver (ie,
second intervals, prefixed with the TSFN's context -- `start`'s receiver (ie,
`new Date()`):

```
Expand Down
8 changes: 4 additions & 4 deletions test/threadsafe_function_ex/threadsafe_function.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ const buildType = process.config.target_defaults.default_configuration;
const assert = require('assert');
const common = require('../common');

module.exports = (async function() {
module.exports = (async function () {
await test(require(`../build/${buildType}/binding.node`));
await test(require(`../build/${buildType}/binding_noexcept.node`));
})();

async function test(binding) {
const expectedArray = (function(arrayLength) {
const expectedArray = (function (arrayLength) {
const result = [];
for (let index = 0; index < arrayLength; index++) {
result.push(arrayLength - 1 - index);
Expand Down Expand Up @@ -60,7 +60,7 @@ async function test(binding) {
});
}
}, false /* abort */, false /* launchSecondary */,
binding.threadsafe_function_ex.MAX_QUEUE_SIZE);
binding.threadsafe_function_ex.MAX_QUEUE_SIZE);
});

// Start the thread in blocking mode, and assert that all values are passed.
Expand Down Expand Up @@ -191,4 +191,4 @@ async function test(binding) {
})).indexOf(0),
-1,
);
}
}

0 comments on commit b54f5eb

Please sign in to comment.