Commit 1676810c by Ali Ijaz Sheikh Committed by Michael Mifsud

fix: propagate async context

This is an alternative to some of the fixes in [1].

Starting with Nan 2.9.0, we have the ability to propagate async context
across async hops. Certain variants of Nan::Callback::Call are now
deprecated to encourage use of the context presevering alternatives.
Certain variants of Node's MakeCallback that were used internally are
going to be deprecated in Node 10.

Summary is that one should use Nan::Call for sync calls, and
Nan::Callback::Call for async. The latter expects an async resource
corresponding to the async operation to be provided at the call time.

This patch fixes things up so that 1) node-sass isn't using any
deprecated APIs, and 2) properly propagates async context for async
callbacks by creating async resources in the appropriate places.

[1]: https://github.com/sass/node-sass/pull/2295
parent 909f6946
...@@ -227,9 +227,16 @@ int GetResult(sass_context_wrapper* ctx_w, Sass_Context* ctx, bool is_sync = fal ...@@ -227,9 +227,16 @@ int GetResult(sass_context_wrapper* ctx_w, Sass_Context* ctx, bool is_sync = fal
return status; return status;
} }
void PerformCall(sass_context_wrapper* ctx_w, Nan::Callback* callback, int argc, v8::Local<v8::Value> argv[]) {
if (ctx_w->is_sync) {
Nan::Call(*callback, argc, argv);
} else {
callback->Call(argc, argv, ctx_w->async_resource);
}
}
void MakeCallback(uv_work_t* req) { void MakeCallback(uv_work_t* req) {
Nan::HandleScope scope; Nan::HandleScope scope;
Nan::AsyncResource async("sass:MakeCallback");
Nan::TryCatch try_catch; Nan::TryCatch try_catch;
sass_context_wrapper* ctx_w = static_cast<sass_context_wrapper*>(req->data); sass_context_wrapper* ctx_w = static_cast<sass_context_wrapper*>(req->data);
...@@ -246,7 +253,7 @@ void MakeCallback(uv_work_t* req) { ...@@ -246,7 +253,7 @@ void MakeCallback(uv_work_t* req) {
if (status == 0 && ctx_w->success_callback) { if (status == 0 && ctx_w->success_callback) {
// if no error, do callback(null, result) // if no error, do callback(null, result)
ctx_w->success_callback->Call(0, 0, &async); PerformCall(ctx_w, ctx_w->success_callback, 0, 0);
} }
else if (ctx_w->error_callback) { else if (ctx_w->error_callback) {
// if error, do callback(error) // if error, do callback(error)
...@@ -254,7 +261,7 @@ void MakeCallback(uv_work_t* req) { ...@@ -254,7 +261,7 @@ void MakeCallback(uv_work_t* req) {
v8::Local<v8::Value> argv[] = { v8::Local<v8::Value> argv[] = {
Nan::New<v8::String>(err).ToLocalChecked() Nan::New<v8::String>(err).ToLocalChecked()
}; };
ctx_w->error_callback->Call(1, argv, &async); PerformCall(ctx_w, ctx_w->error_callback, 1, argv);
} }
if (try_catch.HasCaught()) { if (try_catch.HasCaught()) {
Nan::FatalException(try_catch); Nan::FatalException(try_catch);
...@@ -270,6 +277,8 @@ NAN_METHOD(render) { ...@@ -270,6 +277,8 @@ NAN_METHOD(render) {
struct Sass_Data_Context* dctx = sass_make_data_context(source_string); struct Sass_Data_Context* dctx = sass_make_data_context(source_string);
sass_context_wrapper* ctx_w = sass_make_context_wrapper(); sass_context_wrapper* ctx_w = sass_make_context_wrapper();
ctx_w->async_resource = new Nan::AsyncResource("node-sass:sass_context_wrapper:render");
if (ExtractOptions(options, dctx, ctx_w, false, false) >= 0) { if (ExtractOptions(options, dctx, ctx_w, false, false) >= 0) {
int status = uv_queue_work(uv_default_loop(), &ctx_w->request, compile_it, (uv_after_work_cb)MakeCallback); int status = uv_queue_work(uv_default_loop(), &ctx_w->request, compile_it, (uv_after_work_cb)MakeCallback);
...@@ -304,6 +313,8 @@ NAN_METHOD(render_file) { ...@@ -304,6 +313,8 @@ NAN_METHOD(render_file) {
struct Sass_File_Context* fctx = sass_make_file_context(input_path); struct Sass_File_Context* fctx = sass_make_file_context(input_path);
sass_context_wrapper* ctx_w = sass_make_context_wrapper(); sass_context_wrapper* ctx_w = sass_make_context_wrapper();
ctx_w->async_resource = new Nan::AsyncResource("node-sass:sass_context_wrapper:render_file");
if (ExtractOptions(options, fctx, ctx_w, true, false) >= 0) { if (ExtractOptions(options, fctx, ctx_w, true, false) >= 0) {
int status = uv_queue_work(uv_default_loop(), &ctx_w->request, compile_it, (uv_after_work_cb)MakeCallback); int status = uv_queue_work(uv_default_loop(), &ctx_w->request, compile_it, (uv_after_work_cb)MakeCallback);
......
...@@ -40,6 +40,7 @@ class CallbackBridge { ...@@ -40,6 +40,7 @@ class CallbackBridge {
virtual std::vector<v8::Local<v8::Value>> pre_process_args(std::vector<L>) const =0; virtual std::vector<v8::Local<v8::Value>> pre_process_args(std::vector<L>) const =0;
Nan::Callback* callback; Nan::Callback* callback;
Nan::AsyncResource* async_resource;
bool is_sync; bool is_sync;
uv_mutex_t cv_mutex; uv_mutex_t cv_mutex;
...@@ -66,6 +67,7 @@ CallbackBridge<T, L>::CallbackBridge(v8::Local<v8::Function> callback, bool is_s ...@@ -66,6 +67,7 @@ CallbackBridge<T, L>::CallbackBridge(v8::Local<v8::Function> callback, bool is_s
this->async = new uv_async_t; this->async = new uv_async_t;
this->async->data = (void*) this; this->async->data = (void*) this;
uv_async_init(uv_default_loop(), this->async, (uv_async_cb) dispatched_async_uv_callback); uv_async_init(uv_default_loop(), this->async, (uv_async_cb) dispatched_async_uv_callback);
this->async_resource = new Nan::AsyncResource("node-sass:CallbackBridge");
} }
v8::Local<v8::Function> func = CallbackBridge<T, L>::get_wrapper_constructor().ToLocalChecked(); v8::Local<v8::Function> func = CallbackBridge<T, L>::get_wrapper_constructor().ToLocalChecked();
...@@ -82,6 +84,7 @@ CallbackBridge<T, L>::~CallbackBridge() { ...@@ -82,6 +84,7 @@ CallbackBridge<T, L>::~CallbackBridge() {
if (!is_sync) { if (!is_sync) {
uv_close((uv_handle_t*)this->async, &async_gone); uv_close((uv_handle_t*)this->async, &async_gone);
delete this->async_resource;
} }
} }
...@@ -107,7 +110,7 @@ T CallbackBridge<T, L>::operator()(std::vector<void*> argv) { ...@@ -107,7 +110,7 @@ T CallbackBridge<T, L>::operator()(std::vector<void*> argv) {
argv_v8.push_back(Nan::New(wrapper)); argv_v8.push_back(Nan::New(wrapper));
return this->post_process_return_value( return this->post_process_return_value(
Nan::Call(*callback, argv_v8.size(), &argv_v8[0]).ToLocalChecked() Nan::Call(*this->callback, argv_v8.size(), &argv_v8[0]).ToLocalChecked()
); );
} else { } else {
/* /*
...@@ -151,7 +154,6 @@ void CallbackBridge<T, L>::dispatched_async_uv_callback(uv_async_t *req) { ...@@ -151,7 +154,6 @@ void CallbackBridge<T, L>::dispatched_async_uv_callback(uv_async_t *req) {
* post_process_args(). * post_process_args().
*/ */
Nan::HandleScope scope; Nan::HandleScope scope;
Nan::AsyncResource async("sass:CallbackBridge");
Nan::TryCatch try_catch; Nan::TryCatch try_catch;
std::vector<v8::Local<v8::Value>> argv_v8 = bridge->pre_process_args(bridge->argv); std::vector<v8::Local<v8::Value>> argv_v8 = bridge->pre_process_args(bridge->argv);
...@@ -160,7 +162,7 @@ void CallbackBridge<T, L>::dispatched_async_uv_callback(uv_async_t *req) { ...@@ -160,7 +162,7 @@ void CallbackBridge<T, L>::dispatched_async_uv_callback(uv_async_t *req) {
} }
argv_v8.push_back(Nan::New(bridge->wrapper)); argv_v8.push_back(Nan::New(bridge->wrapper));
bridge->callback->Call(argv_v8.size(), &argv_v8[0], &async); bridge->callback->Call(argv_v8.size(), &argv_v8[0], bridge->async_resource);
if (try_catch.HasCaught()) { if (try_catch.HasCaught()) {
Nan::FatalException(try_catch); Nan::FatalException(try_catch);
......
...@@ -33,6 +33,9 @@ extern "C" { ...@@ -33,6 +33,9 @@ extern "C" {
else if (ctx_w->fctx) { else if (ctx_w->fctx) {
sass_delete_file_context(ctx_w->fctx); sass_delete_file_context(ctx_w->fctx);
} }
if (ctx_w->async_resource) {
delete ctx_w->async_resource;
}
delete ctx_w->error_callback; delete ctx_w->error_callback;
delete ctx_w->success_callback; delete ctx_w->success_callback;
......
...@@ -39,6 +39,7 @@ extern "C" { ...@@ -39,6 +39,7 @@ extern "C" {
// v8 and nan related // v8 and nan related
Nan::Persistent<v8::Object> result; Nan::Persistent<v8::Object> result;
Nan::AsyncResource* async_resource;
Nan::Callback* error_callback; Nan::Callback* error_callback;
Nan::Callback* success_callback; Nan::Callback* success_callback;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment