Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Entering the V8 API without proper locking in place #779

Closed
bsrdjan opened this issue Jul 30, 2020 · 9 comments
Closed

Entering the V8 API without proper locking in place #779

bsrdjan opened this issue Jul 30, 2020 · 9 comments

Comments

@bsrdjan
Copy link

bsrdjan commented Jul 30, 2020

Working on N-API wrapper of an external c++ library, I am getting:

An external library is Entering the V8 API without proper locking in place

The external library instantiates own event listener/dispatcher and provides a c++ callback interface. My callback function, invoked by external lib, should parse the event data and invoke the JavaScript function:

RFC_RC SAP_API genericHandler(RFC_FUNC_NAME func_handle) {

    Server *server = addon_ns::__server;

    ServerFunctionsMap::iterator it = server->serverFunctions.begin();
    while (it != server->serverFunctions.end())
    {
        if (strcmpU(func_name, it->second.func_name) == 0)
        {
            rc = RFC_OK;
            DEBUG("genericHandler found: ", (pointer_t)it->second.func_desc_handle)
            // DEBUG("js handler: ", it->second.callback.Value().ToString().Utf8Value());//  Cannot create a handle w ...
            //
            // invoke JS function here ...
            //
            break;
        }
        it++;
    }
}

Any v8 read access here leads to Cannot create a handle without a HandleScope and the scope can't be created because of: Entering the V8 API without proper locking in place.

Which integration pattern could work in this case?

@gabrielschulhof
Copy link
Contributor

At first blush it sounds like you Napi::Function::MakeCallback if you have a Napi::Function available. It's not sufficient to simply use Napi::Function::Call because the stack is not set up for entering V8.

@bsrdjan
Copy link
Author

bsrdjan commented Aug 3, 2020

Thanks Gabriel, just now reading your great example thread_safe_function_round_trip/napi, trying to make it work in my case :)

The 3rd party lib is listening for external requests, represented by request handle and request data container.

NodeJS

const addon = require("../lib/addon");
const server = new addon.Server();

// Callback function
function myHandler(request_context, jsParameters = {}) {
    console.log("myHandler invoked");
    console.log("request_context", request_context);
    console.log("parameters", jsParameters);
    // some processing ...
    return {
        a: jsParameters.a + 1,
        message: `myHandler here, received B=${jsParameters.B}`,
    };
}

// Server start
server.start((err) => {
    if (err) return console.error("error:", err);
    console.log(
        `Server alive: ${server.alive} client ${server.client_connection} server: ${server.server_connection}`
    );

    // Register the myHandler function as request type 2975 handler
    const REQ_TYPE = 2795
    server.registerJsFunction(REQ_TYPE, myHandler, (err) => {
        if (err) return console.error(`error registering ${REQ_TYPE} handler: ${err}`);
        console.log(`myHandler registered for ${REQ_TYPE}`);
    });
});

// wait for external requests, invoking the `genericHandler` when one received
setTimeout(function () {
    console.log("bye!");
}, 20 * 1000);

Addon

Once the 2795 request received, the genericHandler is invoked, doing following:

    int genericHandler(REQ_TYPE req_type, REQ_DATA req_data)
    {
        // get the server instance from addon namespace, where the Server previously saved it
        Server *server = addon::__server;

        // find the JS handler function
        ServerFunctionsMap::iterator it = server->serverFunctions.begin();
        while (it != server->serverFunctions.end())
        {
            if (it.req_type == req_type)
            {
                printf("found req_type %u\n"); 
                break;
            }
            it++;
        }

        if (it == server->serverFunctions.end())
        {
            printf("not found!\n");
            return 1;
        }

        // here the jsHandle Function Reference is available in iterator it (registered by registerJsFunction ...) and 
        // and next steps should be:

        //
        // 1. Unpack the req_data and create JS parameters' values
        // Napi::Value jsParameters = extToJS(req_data)
       
        //
        // 2. Call the jsHandle function, updating parameters' values, something like:
        //
        // jsHandle.Call({jsParameters});

        //
        // 3. Parse the jsHandle results, writing back to req_data
        //
        // req_data.result = JStoExt(jsParameters)

        // let external dispatcher know all went fine, so the response (updated req_data container), can be sent back
        return RFC_OK;
    }

The little problem is that genericHandler callback is invoked out of JS context / event loop and even the step 1 (unpack) can't start here, thus still far from making a callback. Based on my current understanding, something like round_trip.c could work here and have the following question.

The genericHandler could, like the PrimeThread in your example, pass the req_data to thread safe jsHandle function but threads in your example interact asynchronously, while my genericHandler c++ callback, as defined by external lib, is running in own thread, expecting the synchronous answer there. That being said, still no idea if and how this integration could work the napi way and any hints/ideas welcome.

@bsrdjan
Copy link
Author

bsrdjan commented Aug 24, 2020

Any more ideas / thoughts how this scenario could work, if possible at all ?

The C callback handler is externally triggered, outside of Node event loop. Once triggered, it should acquire the JS context, synchronise with event loop, call the JS function and return the result to callback handler.

@mhdawson
Copy link
Member

@bsrdjan I'm not sure I understand the issue 100%, but the only problem that would seem to remain if you use the pattern in the thread_safe_function_round_trip/napi example is that you want the genericHandler to wait for the result of the

CHECK(napi_call_function(env,
                              undefined,
                              js_cb,
                              1,
                              &js_thread_item,
                              NULL) == napi_ok);

call

My thought is that you just need to update the code to add your own synchronization between the 2 threads to make genericHandler wait for the response. That would be to add a wait after the napi_call_threadsafe_function in the genericHandler and a notify after the napi_call_function in the CallJs method in the example.

In line with the example you should be able to use a uv_cond for that or if you want to preserve full abi stability native locks/cond vars in order to avoid the libuv dep.

@bsrdjan
Copy link
Author

bsrdjan commented Sep 3, 2020

Thanks @mhdawson, it makes sense. Will give it a try.

@bsrdjan bsrdjan closed this as completed Sep 3, 2020
@lilong7676
Copy link

Thanks @mhdawson, it makes sense. Will give it a try.

I have a similar problem. How did you solve it? Any code snippets? thank you

@lilong7676
Copy link

@bsrdjan

@bsrdjan
Copy link
Author

bsrdjan commented Dec 16, 2020

@lilong7676, the best code snippet I could find is the thread_safe_function_round_trip/napi example. Applicable in my case but not a priority for me, for time being. Have therefore nothing more to share.

@lilong7676
Copy link

@bsrdjan Thank you. I solved this problem with ThreadSafeFunction

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants