Context parameter on thread safe function callback

[legendecas]

Currently Napi::ThreadSafeFunction accepts a type implementing void operator()(Napi::Env env, Function jsCallback, DataType* data). It would be hard to retrieve the initial context parameter on Napi::ThreadSafeFunction::New since there is only one parameter DataType* data or we have to wrap the data with a reference to context again.

[legendecas]

As a reference napi_threadsafe_function_call_js has a signature of

typedef void (*napi_threadsafe_function_call_js)(napi_env env,
                                                 napi_value js_callback,
                                                 void* context,
                                                 void* data);

[KevinEady]

Looking at the signature for our CallJS wrapper passed to napi_create_threadsafe_function:

node-addon-api/napi-inl.h

Lines 4572 to 4575 in 4e88506

	inline void ThreadSafeFunction::CallJS(napi_env env,
	napi_value jsCallback,
	void* /* context */,
	void* data) {

... we completely ignore the context value.

The context is not wrapped, so one could use ThreadSafeFunction::GetContext() but it is an additional call that shouldn't be necessary.

[legendecas]

The context is not wrapped, so one could use ThreadSafeFunction::GetContext() but it is an additional call that shouldn't be necessary.

I've considered this option too. It seems on the user callback we don't have the reference to call ThreadSafeFunction::GetContext() if we don't wrap it in the data. So the option still relies on wrapping around data.

[gabrielschulhof]

From #594 (comment) it looks like we may need to substantially revamp Napi::ThreadSafeFunction. When we do so, we should also address this issue.

[KevinEady]

Sooo I have been working on this for some time now... After struggling with templated std::functions for hours, I really see two ways of implementing:

1. Allow the existing [Non]BlockingCall()s to take a three-argument callback, with the context being void*.
2. Create a new function [Non]BlockingCallEx which requires a three argument callback. I think I can make the context argument templated T *... but since the context is stored on the underlying napi_threadsafe_function and not the call itself, the node-addon-api wrapper to run the callback void(Env, Function, T*) would perform a cast to T* on the context, since it's void*.

I dunno, I am leaning towards option 1... i'm sure there are other options but there was no way I could figure out how to "combine" the two options (modify the existing callbacks to allow templated argument third T* context)...

Thoughts?

cc: @gabrielschulhof @legendecas

[github-actions]

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.