From 0870c4a1410b50d20b62e1621e2240eefebde94c Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 17 Nov 2017 22:13:18 +0100 Subject: [PATCH 1/4] src: add helper for addons to get the event loop Add a utility functions for addons to use when they need a reference to the current event loop. Currently, `uv_default_loop()` works if the embedder is the single-threaded default node executable, but even without the presence of e.g. workers that might not really an API guarantee for when Node is embedded. --- src/node.cc | 5 +++++ src/node.h | 2 ++ test/addons/async-hello-world/binding.cc | 2 +- test/addons/callback-scope/binding.cc | 5 ++++- 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/node.cc b/src/node.cc index a1d5ad7e254469..0b115c0b1649bc 100644 --- a/src/node.cc +++ b/src/node.cc @@ -4419,6 +4419,11 @@ void RunAtExit(Environment* env) { } +uv_loop_t* GetCurrentEventLoop(v8::Isolate* isolate) { + return Environment::GetCurrent(isolate)->event_loop(); +} + + static uv_key_t thread_local_env; diff --git a/src/node.h b/src/node.h index 1d4583bbccd8d3..ac14f4b69c1cc9 100644 --- a/src/node.h +++ b/src/node.h @@ -248,6 +248,8 @@ NODE_EXTERN void EmitBeforeExit(Environment* env); NODE_EXTERN int EmitExit(Environment* env); NODE_EXTERN void RunAtExit(Environment* env); +NODE_EXTERN struct uv_loop_s* GetCurrentEventLoop(v8::Isolate* isolate); + /* Converts a unixtime to V8 Date */ #define NODE_UNIXTIME_V8(t) v8::Date::New(v8::Isolate::GetCurrent(), \ 1000 * static_cast(t)) diff --git a/test/addons/async-hello-world/binding.cc b/test/addons/async-hello-world/binding.cc index f773bfffcdb95d..5b3a800709f7d7 100644 --- a/test/addons/async-hello-world/binding.cc +++ b/test/addons/async-hello-world/binding.cc @@ -77,7 +77,7 @@ void Method(const v8::FunctionCallbackInfo& args) { v8::Local callback = v8::Local::Cast(args[1]); req->callback.Reset(isolate, callback); - uv_queue_work(uv_default_loop(), + uv_queue_work(node::GetCurrentEventLoop(isolate), &req->req, DoAsync, (uv_after_work_cb)AfterAsync); diff --git a/test/addons/callback-scope/binding.cc b/test/addons/callback-scope/binding.cc index f03d8a20d5333f..34d452bbb7c710 100644 --- a/test/addons/callback-scope/binding.cc +++ b/test/addons/callback-scope/binding.cc @@ -52,7 +52,10 @@ static void TestResolveAsync(const v8::FunctionCallbackInfo& args) { uv_work_t* req = new uv_work_t; - uv_queue_work(uv_default_loop(), req, [](uv_work_t*) {}, Callback); + uv_queue_work(node::GetCurrentEventLoop(isolate), + req, + [](uv_work_t*) {}, + Callback); } v8::Local local = From e5af5c0219e3033a6dacf1b1cf565d5ca85df06a Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 17 Nov 2017 22:16:43 +0100 Subject: [PATCH 2/4] n-api: add helper for addons to get the event loop Add a utility functions for addons to use when they need a reference to the current event loop. While the libuv API is not directly part of N-API, it provides a quite stable C API as well, and is tightly integrated with Node itself. As a particular use case, without access to the event loop it is hard to do something interesting from inside a N-API finalizer function, since calls into JS and therefore virtually all other N-API functions are not allowed. --- doc/api/n-api.md | 17 ++++ src/node_api.cc | 19 +++-- src/node_api.h | 6 ++ test/addons-napi/test_general/test.js | 2 +- test/addons-napi/test_uv_loop/binding.gyp | 8 ++ test/addons-napi/test_uv_loop/test.js | 5 ++ test/addons-napi/test_uv_loop/test_uv_loop.cc | 79 +++++++++++++++++++ 7 files changed, 129 insertions(+), 7 deletions(-) create mode 100644 test/addons-napi/test_uv_loop/binding.gyp create mode 100644 test/addons-napi/test_uv_loop/test.js create mode 100644 test/addons-napi/test_uv_loop/test_uv_loop.cc diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 7852454a9295c5..dd5359ec2cfc6b 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -3684,6 +3684,23 @@ NAPI_EXTERN napi_status napi_run_script(napi_env env, - `[in] script`: A JavaScript string containing the script to execute. - `[out] result`: The value resulting from having executed the script. +## libuv event loop + +N-API provides a function for getting the current event loop associated with +a specific `napi_env`. + +### napi_uv_get_event_loop + +```C +NAPI_EXTERN napi_status napi_get_uv_event_loop(napi_env env, + uv_loop_t** loop); +``` + +- `[in] env`: The environment that the API is invoked under. +- `[out] loop`: The current libuv loop instance. + [Promises]: #n_api_promises [Simple Asynchronous Operations]: #n_api_simple_asynchronous_operations [Custom Asynchronous Operations]: #n_api_custom_asynchronous_operations diff --git a/src/node_api.cc b/src/node_api.cc index ad922d9ad195db..430d51d0fc3c8f 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -18,7 +18,7 @@ #include "node_api.h" #include "node_internals.h" -#define NAPI_VERSION 1 +#define NAPI_VERSION 2 static napi_status napi_set_last_error(napi_env env, napi_status error_code, @@ -3401,15 +3401,22 @@ napi_status napi_delete_async_work(napi_env env, napi_async_work work) { return napi_clear_last_error(env); } +napi_status napi_get_uv_event_loop(napi_env env, uv_loop_t** loop) { + CHECK_ENV(env); + CHECK_ARG(env, loop); + *loop = node::GetCurrentEventLoop(env->isolate); + return napi_clear_last_error(env); +} + napi_status napi_queue_async_work(napi_env env, napi_async_work work) { CHECK_ENV(env); CHECK_ARG(env, work); - // Consider: Encapsulate the uv_loop_t into an opaque pointer parameter. - // Currently the environment event loop is the same as the UV default loop. - // Someday (if node ever supports multiple isolates), it may be better to get - // the loop from node::Environment::GetCurrent(env->isolate)->event_loop(); - uv_loop_t* event_loop = uv_default_loop(); + napi_status status; + uv_loop_t* event_loop = nullptr; + status = napi_get_uv_event_loop(env, &event_loop); + if (status != napi_ok) + return napi_set_last_error(env, status); uvimpl::Work* w = reinterpret_cast(work); diff --git a/src/node_api.h b/src/node_api.h index a3a07a64673366..8e5eef8a47728f 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -14,6 +14,8 @@ #include #include "node_api_types.h" +struct uv_loop_s; // Forward declaration. + #ifdef _WIN32 #ifdef BUILDING_NODE_EXTENSION #ifdef EXTERNAL_NAPI @@ -581,6 +583,10 @@ NAPI_EXTERN napi_status napi_run_script(napi_env env, napi_value script, napi_value* result); +// Return the current libuv event loop for a given environment +NAPI_EXTERN napi_status napi_get_uv_event_loop(napi_env env, + struct uv_loop_s** loop); + EXTERN_C_END #endif // SRC_NODE_API_H_ diff --git a/test/addons-napi/test_general/test.js b/test/addons-napi/test_general/test.js index 3ff57922c5372b..ee6618c8121289 100644 --- a/test/addons-napi/test_general/test.js +++ b/test/addons-napi/test_general/test.js @@ -34,7 +34,7 @@ assert.ok(test_general.testGetPrototype(baseObject) !== // test version management functions // expected version is currently 1 -assert.strictEqual(test_general.testGetVersion(), 1); +assert.strictEqual(test_general.testGetVersion(), 2); const [ major, minor, patch, release ] = test_general.testGetNodeVersion(); assert.strictEqual(process.version.split('-')[0], diff --git a/test/addons-napi/test_uv_loop/binding.gyp b/test/addons-napi/test_uv_loop/binding.gyp new file mode 100644 index 00000000000000..81fcfdc592a523 --- /dev/null +++ b/test/addons-napi/test_uv_loop/binding.gyp @@ -0,0 +1,8 @@ +{ + "targets": [ + { + "target_name": "test_uv_loop", + "sources": [ "test_uv_loop.cc" ] + } + ] +} diff --git a/test/addons-napi/test_uv_loop/test.js b/test/addons-napi/test_uv_loop/test.js new file mode 100644 index 00000000000000..4efc3c6fcd7001 --- /dev/null +++ b/test/addons-napi/test_uv_loop/test.js @@ -0,0 +1,5 @@ +'use strict'; +const common = require('../../common'); +const { SetImmediate } = require(`./build/${common.buildType}/test_uv_loop`); + +SetImmediate(common.mustCall()); diff --git a/test/addons-napi/test_uv_loop/test_uv_loop.cc b/test/addons-napi/test_uv_loop/test_uv_loop.cc new file mode 100644 index 00000000000000..c95be3be116541 --- /dev/null +++ b/test/addons-napi/test_uv_loop/test_uv_loop.cc @@ -0,0 +1,79 @@ +#include +#include +#include +#include +#include +#include "../common.h" + +template +void* SetImmediate(napi_env env, T&& cb) { + T* ptr = new T(std::move(cb)); + uv_loop_t* loop = nullptr; + uv_check_t* check = new uv_check_t; + check->data = static_cast(ptr); + NAPI_ASSERT(env, + napi_get_uv_event_loop(env, &loop) == napi_ok, + "can get event loop"); + uv_check_init(loop, check); + uv_check_start(check, [](uv_check_t* check) { + std::unique_ptr ptr {static_cast(check->data)}; + T cb = std::move(*ptr); + uv_check_stop(check); + uv_close(reinterpret_cast(check), [](uv_handle_t* handle) { + delete reinterpret_cast(handle); + }); + + assert(cb() != nullptr); + }); + return nullptr; +} + +static char dummy; + +napi_value SetImmediateBinding(napi_env env, napi_callback_info info) { + size_t argc = 1; + napi_value argv[1]; + napi_value _this; + void* data; + NAPI_CALL(env, + napi_get_cb_info(env, info, &argc, argv, &_this, &data)); + NAPI_ASSERT(env, argc >= 1, "Not enough arguments, expected 1."); + + napi_valuetype t; + NAPI_CALL(env, napi_typeof(env, argv[0], &t)); + NAPI_ASSERT(env, t == napi_function, + "Wrong first argument, function expected."); + + napi_ref cbref; + NAPI_CALL(env, + napi_create_reference(env, argv[0], 1, &cbref)); + + SetImmediate(env, [=]() -> char* { + napi_value undefined; + napi_value callback; + napi_handle_scope scope; + NAPI_CALL(env, napi_open_handle_scope(env, &scope)); + NAPI_CALL(env, napi_get_undefined(env, &undefined)); + NAPI_CALL(env, napi_get_reference_value(env, cbref, &callback)); + NAPI_CALL(env, napi_delete_reference(env, cbref)); + NAPI_CALL(env, + napi_call_function(env, undefined, callback, 0, nullptr, nullptr)); + NAPI_CALL(env, napi_close_handle_scope(env, scope)); + return &dummy; + }); + + return nullptr; +} + +napi_value Init(napi_env env, napi_value exports) { + napi_property_descriptor properties[] = { + DECLARE_NAPI_PROPERTY("SetImmediate", SetImmediateBinding) + }; + + NAPI_CALL(env, napi_define_properties( + env, exports, sizeof(properties) / sizeof(*properties), properties)); + + return exports; +} + +NAPI_MODULE(NODE_GYP_MODULE_NAME, Init) From 421f9d1d178e426b773cac15e9e1a1f34830b23b Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 18 Nov 2017 14:01:51 +0100 Subject: [PATCH 3/4] [squash] bnoordhuis comments --- src/node.cc | 6 +++++- src/node.h | 2 ++ src/node_api.cc | 11 +++++++---- test/addons-napi/test_uv_loop/test_uv_loop.cc | 5 ++--- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/node.cc b/src/node.cc index 0b115c0b1649bc..de03b89e16623b 100644 --- a/src/node.cc +++ b/src/node.cc @@ -4420,7 +4420,11 @@ void RunAtExit(Environment* env) { uv_loop_t* GetCurrentEventLoop(v8::Isolate* isolate) { - return Environment::GetCurrent(isolate)->event_loop(); + HandleScope handle_scope(isolate); + auto context = isolate->GetCurrentContext(); + if (context.IsEmpty()) + return nullptr; + return Environment::GetCurrent(context)->event_loop(); } diff --git a/src/node.h b/src/node.h index ac14f4b69c1cc9..9bc317ca04edfa 100644 --- a/src/node.h +++ b/src/node.h @@ -248,6 +248,8 @@ NODE_EXTERN void EmitBeforeExit(Environment* env); NODE_EXTERN int EmitExit(Environment* env); NODE_EXTERN void RunAtExit(Environment* env); +// This may return nullptr if the current v8::Context is not associated +// with a Node instance. NODE_EXTERN struct uv_loop_s* GetCurrentEventLoop(v8::Isolate* isolate); /* Converts a unixtime to V8 Date */ diff --git a/src/node_api.cc b/src/node_api.cc index 430d51d0fc3c8f..84e57d03d6237d 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -28,8 +28,10 @@ static napi_status napi_clear_last_error(napi_env env); struct napi_env__ { - explicit napi_env__(v8::Isolate* _isolate): isolate(_isolate), - last_error() {} + explicit napi_env__(v8::Isolate* _isolate, uv_loop_t* _loop) + : isolate(_isolate), + last_error(), + loop(_loop) {} ~napi_env__() { last_exception.Reset(); wrap_template.Reset(); @@ -43,6 +45,7 @@ struct napi_env__ { v8::Persistent accessor_data_template; napi_extended_error_info last_error; int open_handle_scopes = 0; + uv_loop_t* loop = nullptr; }; #define ENV_OBJECT_TEMPLATE(env, prefix, destination, field_count) \ @@ -771,7 +774,7 @@ napi_env GetEnv(v8::Local context) { if (value->IsExternal()) { result = static_cast(value.As()->Value()); } else { - result = new napi_env__(isolate); + result = new napi_env__(isolate, node::GetCurrentEventLoop(isolate)); auto external = v8::External::New(isolate, result); // We must also stop hard if the result of assigning the env to the global @@ -3404,7 +3407,7 @@ napi_status napi_delete_async_work(napi_env env, napi_async_work work) { napi_status napi_get_uv_event_loop(napi_env env, uv_loop_t** loop) { CHECK_ENV(env); CHECK_ARG(env, loop); - *loop = node::GetCurrentEventLoop(env->isolate); + *loop = env->loop; return napi_clear_last_error(env); } diff --git a/test/addons-napi/test_uv_loop/test_uv_loop.cc b/test/addons-napi/test_uv_loop/test_uv_loop.cc index c95be3be116541..44819f72bb6b9d 100644 --- a/test/addons-napi/test_uv_loop/test_uv_loop.cc +++ b/test/addons-napi/test_uv_loop/test_uv_loop.cc @@ -5,12 +5,12 @@ #include #include "../common.h" -template +template void* SetImmediate(napi_env env, T&& cb) { T* ptr = new T(std::move(cb)); uv_loop_t* loop = nullptr; uv_check_t* check = new uv_check_t; - check->data = static_cast(ptr); + check->data = ptr; NAPI_ASSERT(env, napi_get_uv_event_loop(env, &loop) == napi_ok, "can get event loop"); @@ -18,7 +18,6 @@ void* SetImmediate(napi_env env, T&& cb) { uv_check_start(check, [](uv_check_t* check) { std::unique_ptr ptr {static_cast(check->data)}; T cb = std::move(*ptr); - uv_check_stop(check); uv_close(reinterpret_cast(check), [](uv_handle_t* handle) { delete reinterpret_cast(handle); }); From 681244904418c317d915655831b28df57f845816 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 20 Nov 2017 18:27:10 +0100 Subject: [PATCH 4/4] [squash] cjihrig comment --- doc/api/n-api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index dd5359ec2cfc6b..6c34f21151219d 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -3689,7 +3689,7 @@ NAPI_EXTERN napi_status napi_run_script(napi_env env, N-API provides a function for getting the current event loop associated with a specific `napi_env`. -### napi_uv_get_event_loop +### napi_get_uv_event_loop