Skip to content

Commit 807fb27

Browse files
Apply suggestions from code review
Co-authored-by: Gabriel Schulhof <[email protected]>
1 parent c8eea6b commit 807fb27

File tree

3 files changed

+14
-14
lines changed

3 files changed

+14
-14
lines changed

doc/threadsafe.md

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ easy way to do this. These APIs provide two types --
1414
[`Napi::ThreadSafeFunctionEx`](threadsafe_function_ex.md) -- as well as APIs to
1515
create, destroy, and call objects of this type. The differences between the two
1616
are subtle and are [highlighted below](#implementation-differences). Regardless
17-
of which type you choose, the API between the two are similar.
17+
of which type you choose, the APIs between the two are similar.
1818

1919
`Napi::ThreadSafeFunction[Ex]::New()` creates a persistent reference that holds
2020
a JavaScript function which can be called from multiple threads. The calls
@@ -44,11 +44,11 @@ reaches zero, no further threads can start making use of it by calling
4444

4545
The choice between `Napi::ThreadSafeFunction` and `Napi::ThreadSafeFunctionEx`
4646
depends largely on how you plan to execute your native C++ code (the "callback")
47-
on the Node thread.
47+
on the Node.js thread.
4848

4949
### [`Napi::ThreadSafeFunction`](threadsafe_function.md)
5050

51-
This API is designed without N-API 5 native support for [optional JavaScript
51+
This API is designed without N-API 5 native support for [the optional JavaScript
5252
function callback feature](https://github.com/nodejs/node/commit/53297e66cb).
5353
`::New` methods that do not have a `Function` parameter will construct a
5454
_new_, no-op `Function` on the environment to pass to the underlying N-API
@@ -62,12 +62,12 @@ This API has some dynamic functionality, in that:
6262
- Different C++ data types may be passed with each call of `[Non]BlockingCall()`
6363
to match the specific data type as specified in the `CallJs` callback.
6464

65-
However, this functionality comes with some **additional overhead** and
65+
Note that this functionality comes with some **additional overhead** and
6666
situational **memory leaks**:
67-
- The API acts as a "middle-man" between the underlying
67+
- The API acts as a "broker" between the underlying
6868
`napi_threadsafe_function`, and dynamically constructs a wrapper for your
6969
callback on the heap for every call to `[Non]BlockingCall()`.
70-
- In acting in this "middle-man" fashion, the API will call the underlying "make
70+
- In acting in this "broker" fashion, the API will call the underlying "make
7171
call" N-API method on this packaged item. If the API has determined the
7272
thread-safe function is no longer accessible (eg. all threads have released yet
7373
there are still items on the queue), **the callback passed to
@@ -88,15 +88,15 @@ drawbacks listed above. The API is designed with N-API 5's support of an
8888
optional function callback. The API will correctly allow developers to pass
8989
`std::nullptr` instead of a `const Function&` for the callback function
9090
specified in `::New`. It also provides helper APIs to _target_ N-API 4 and
91-
construct a no-op `Function` **or** to target N-API 5 and "construct" an
91+
construct a no-op `Function` **or** to target N-API 5 and "construct" a
9292
`std::nullptr` callback. This allows a single codebase to use the same APIs,
9393
with just a switch of the `NAPI_VERSION` compile-time constant.
9494

95-
The removal of the dynamic call functionality has the additional side effects:
96-
- The API does _not_ act as a "middle-man" compared to the non-`Ex`. Once Node
95+
The removal of the dynamic call functionality has the following implications:
96+
- The API does _not_ act as a "broker" compared to the non-`Ex`. Once Node.js
9797
finalizes the thread-safe function, the `CallJs` callback will execute with an
98-
empty `Napi::Env` for any remaining items on the queue. This provides the the
99-
ability to handle any necessary clean up of the item's data.
98+
empty `Napi::Env` for any remaining items on the queue. This provides the
99+
ability to handle any necessary cleanup of the item's data.
100100
- The callback _does_ receive the context as a parameter, so a call to
101101
`GetContext()` is _not_ necessary. This context type is specified as the
102102
**first type argument** specified to `::New`, ensuring type safety.

doc/threadsafe_function.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ New(napi_env env,
6262
- `initialThreadCount`: The initial number of threads, including the main
6363
thread, which will be making use of this function.
6464
- `[optional] context`: Data to attach to the resulting `ThreadSafeFunction`.
65-
Can be retreived via `GetContext()`.
65+
It can be retreived by calling `GetContext()`.
6666
- `[optional] finalizeCallback`: Function to call when the `ThreadSafeFunction`
6767
is being destroyed. This callback will be invoked on the main thread when the
6868
thread-safe function is about to be destroyed. It receives the context and the

doc/threadsafe_function_ex.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ of:
88
a TSFN has no context.
99
- `DataType = void*`: The data to use in the native callback. By default, a TSFN
1010
can accept *any* data type.
11-
- `Callback = void*(Napi::Env, Napi::Function jsCallback, ContextType*,
11+
- `Callback = void(*)(Napi::Env, Napi::Function jsCallback, ContextType*,
1212
DataType*)`: The callback to run for each item added to the queue. If no
1313
`Callback` is given, the API will call the function `jsCallback` with no
1414
arguments.
@@ -73,7 +73,7 @@ New(napi_env env,
7373
- `initialThreadCount`: The initial number of threads, including the main
7474
thread, which will be making use of this function.
7575
- `[optional] context`: Data to attach to the resulting `ThreadSafeFunction`.
76-
Can be retreived via `GetContext()`.
76+
It can be retreived via `GetContext()`.
7777
- `[optional] finalizeCallback`: Function to call when the
7878
`ThreadSafeFunctionEx` is being destroyed. This callback will be invoked on
7979
the main thread when the thread-safe function is about to be destroyed. It

0 commit comments

Comments
 (0)