diff options
author | Andreas Madsen <amwebdk@gmail.com> | 2017-11-22 18:41:00 +0100 |
---|---|---|
committer | Andreas Madsen <amwebdk@gmail.com> | 2017-12-19 18:04:52 +0100 |
commit | 3b8da4cbe8a7f36fcd8892c6676a55246ba8c3be (patch) | |
tree | 1afc84002d1716f4acd28126df214127e0262c3c /src | |
parent | 0784b0440c05464f79b857f7d8698fcc953d3fb3 (diff) | |
download | node-new-3b8da4cbe8a7f36fcd8892c6676a55246ba8c3be.tar.gz |
async_hooks: use scope for defaultTriggerAsyncId
Previously the getter would mutate the kDefaultTriggerAsncId value. This
refactor changes the setter to bind the current kDefaultTriggerAsncId to
a scope, such that the getter doesn't have to mutate its own value.
PR-URL: https://github.com/nodejs/node/pull/17273
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Diffstat (limited to 'src')
-rw-r--r-- | src/async_wrap.cc | 11 | ||||
-rw-r--r-- | src/connection_wrap.cc | 1 | ||||
-rw-r--r-- | src/env-inl.h | 33 | ||||
-rw-r--r-- | src/env.h | 20 | ||||
-rw-r--r-- | src/pipe_wrap.cc | 3 | ||||
-rw-r--r-- | src/stream_base-inl.h | 3 | ||||
-rw-r--r-- | src/stream_base.cc | 38 | ||||
-rw-r--r-- | src/tcp_wrap.cc | 9 | ||||
-rw-r--r-- | src/udp_wrap.cc | 12 |
9 files changed, 73 insertions, 57 deletions
diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 555d616426..79d54b1000 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -308,12 +308,13 @@ static void PromiseHook(PromiseHookType type, Local<Promise> promise, if (parent_wrap == nullptr) { parent_wrap = PromiseWrap::New(env, parent_promise, nullptr, true); } - // get id from parentWrap - double trigger_async_id = parent_wrap->get_async_id(); - env->set_default_trigger_async_id(trigger_async_id); - } - wrap = PromiseWrap::New(env, promise, parent_wrap, silent); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope( + env, parent_wrap->get_async_id()); + wrap = PromiseWrap::New(env, promise, parent_wrap, silent); + } else { + wrap = PromiseWrap::New(env, promise, nullptr, silent); + } } CHECK_NE(wrap, nullptr); diff --git a/src/connection_wrap.cc b/src/connection_wrap.cc index b7c1949e11..8de77f361d 100644 --- a/src/connection_wrap.cc +++ b/src/connection_wrap.cc @@ -49,7 +49,6 @@ void ConnectionWrap<WrapType, UVType>::OnConnection(uv_stream_t* handle, }; if (status == 0) { - env->set_default_trigger_async_id(wrap_data->get_async_id()); // Instantiate the client javascript object and handle. Local<Object> client_obj = WrapType::Instantiate(env, wrap_data, diff --git a/src/env-inl.h b/src/env-inl.h index 2d4c0c18e9..1451631728 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -176,23 +176,27 @@ inline void Environment::AsyncHooks::clear_async_id_stack() { async_id_fields_[kTriggerAsyncId] = 0; } -inline Environment::AsyncHooks::InitScope::InitScope( - Environment* env, double init_trigger_async_id) - : env_(env), - async_id_fields_ref_(env->async_hooks()->async_id_fields()) { - if (env_->async_hooks()->fields()[AsyncHooks::kCheck] > 0) { - CHECK_GE(init_trigger_async_id, -1); +inline Environment::AsyncHooks::DefaultTriggerAsyncIdScope + ::DefaultTriggerAsyncIdScope(Environment* env, + double default_trigger_async_id) + : async_id_fields_ref_(env->async_hooks()->async_id_fields()) { + if (env->async_hooks()->fields()[AsyncHooks::kCheck] > 0) { + CHECK_GE(default_trigger_async_id, 0); } - env->async_hooks()->push_async_ids( - async_id_fields_ref_[AsyncHooks::kExecutionAsyncId], - init_trigger_async_id); + + old_default_trigger_async_id_ = + async_id_fields_ref_[AsyncHooks::kDefaultTriggerAsyncId]; + async_id_fields_ref_[AsyncHooks::kDefaultTriggerAsyncId] = + default_trigger_async_id; } -inline Environment::AsyncHooks::InitScope::~InitScope() { - env_->async_hooks()->pop_async_id( - async_id_fields_ref_[AsyncHooks::kExecutionAsyncId]); +inline Environment::AsyncHooks::DefaultTriggerAsyncIdScope + ::~DefaultTriggerAsyncIdScope() { + async_id_fields_ref_[AsyncHooks::kDefaultTriggerAsyncId] = + old_default_trigger_async_id_; } + inline Environment::AsyncCallbackScope::AsyncCallbackScope(Environment* env) : env_(env) { env_->makecallback_cntr_++; @@ -456,17 +460,12 @@ inline double Environment::trigger_async_id() { inline double Environment::get_default_trigger_async_id() { double default_trigger_async_id = async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId]; - async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId] = -1; // If defaultTriggerAsyncId isn't set, use the executionAsyncId if (default_trigger_async_id < 0) default_trigger_async_id = execution_async_id(); return default_trigger_async_id; } -inline void Environment::set_default_trigger_async_id(const double id) { - async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId] = id; -} - inline double* Environment::heap_statistics_buffer() const { CHECK_NE(heap_statistics_buffer_, nullptr); return heap_statistics_buffer_; @@ -402,22 +402,23 @@ class Environment { inline size_t stack_size(); inline void clear_async_id_stack(); // Used in fatal exceptions. - // Used to propagate the trigger_async_id to the constructor of any newly - // created resources using RAII. Instead of needing to pass the - // trigger_async_id along with other constructor arguments. - class InitScope { + // Used to set the kDefaultTriggerAsyncId in a scope. This is instead of + // passing the trigger_async_id along with other constructor arguments. + class DefaultTriggerAsyncIdScope { public: - InitScope() = delete; - explicit InitScope(Environment* env, double init_trigger_async_id); - ~InitScope(); + DefaultTriggerAsyncIdScope() = delete; + explicit DefaultTriggerAsyncIdScope(Environment* env, + double init_trigger_async_id); + ~DefaultTriggerAsyncIdScope(); private: - Environment* env_; AliasedBuffer<double, v8::Float64Array> async_id_fields_ref_; + double old_default_trigger_async_id_; - DISALLOW_COPY_AND_ASSIGN(InitScope); + DISALLOW_COPY_AND_ASSIGN(DefaultTriggerAsyncIdScope); }; + private: friend class Environment; // So we can call the constructor. inline explicit AsyncHooks(v8::Isolate* isolate); @@ -559,7 +560,6 @@ class Environment { inline double execution_async_id(); inline double trigger_async_id(); inline double get_default_trigger_async_id(); - inline void set_default_trigger_async_id(const double id); // List of id's that have been destroyed and need the destroy() cb called. inline std::vector<double>* destroy_async_id_list(); diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc index c6dce756ce..c5958a2271 100644 --- a/src/pipe_wrap.cc +++ b/src/pipe_wrap.cc @@ -53,7 +53,8 @@ Local<Object> PipeWrap::Instantiate(Environment* env, AsyncWrap* parent, PipeWrap::SocketType type) { EscapableHandleScope handle_scope(env->isolate()); - AsyncHooks::InitScope init_scope(env, parent->get_async_id()); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(env, + parent->get_async_id()); CHECK_EQ(false, env->pipe_constructor_template().IsEmpty()); Local<Function> constructor = env->pipe_constructor_template()->GetFunction(); CHECK_EQ(false, constructor.IsEmpty()); diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h index cc89a11bac..cdcff67cc5 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -143,7 +143,8 @@ void StreamBase::JSMethod(const FunctionCallbackInfo<Value>& args) { if (!wrap->IsAlive()) return args.GetReturnValue().Set(UV_EINVAL); - AsyncHooks::InitScope init_scope(handle->env(), handle->get_async_id()); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope( + handle->env(), handle->get_async_id()); args.GetReturnValue().Set((wrap->*Method)(args)); } diff --git a/src/stream_base.cc b/src/stream_base.cc index f4bfb72f3f..b1aea79d52 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -52,7 +52,7 @@ int StreamBase::Shutdown(const FunctionCallbackInfo<Value>& args) { AsyncWrap* wrap = GetAsyncWrap(); CHECK_NE(wrap, nullptr); - env->set_default_trigger_async_id(wrap->get_async_id()); + AsyncHooks::DefaultTriggerAsyncIdScope(env, wrap->get_async_id()); ShutdownWrap* req_wrap = new ShutdownWrap(env, req_wrap_obj, this); @@ -109,7 +109,6 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) { size_t storage_size = 0; uint32_t bytes = 0; size_t offset; - AsyncWrap* wrap; WriteWrap* req_wrap; int err; @@ -153,10 +152,13 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) { goto done; } - wrap = GetAsyncWrap(); - CHECK_NE(wrap, nullptr); - env->set_default_trigger_async_id(wrap->get_async_id()); - req_wrap = WriteWrap::New(env, req_wrap_obj, this, storage_size); + { + AsyncWrap* wrap = GetAsyncWrap(); + CHECK_NE(wrap, nullptr); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(env, + wrap->get_async_id()); + req_wrap = WriteWrap::New(env, req_wrap_obj, this, storage_size); + } offset = 0; if (!all_buffers) { @@ -227,7 +229,6 @@ int StreamBase::WriteBuffer(const FunctionCallbackInfo<Value>& args) { const char* data = Buffer::Data(args[1]); size_t length = Buffer::Length(args[1]); - AsyncWrap* wrap; WriteWrap* req_wrap; uv_buf_t buf; buf.base = const_cast<char*>(data); @@ -243,11 +244,14 @@ int StreamBase::WriteBuffer(const FunctionCallbackInfo<Value>& args) { goto done; CHECK_EQ(count, 1); - wrap = GetAsyncWrap(); - if (wrap != nullptr) - env->set_default_trigger_async_id(wrap->get_async_id()); // Allocate, or write rest - req_wrap = WriteWrap::New(env, req_wrap_obj, this); + { + AsyncWrap* wrap = GetAsyncWrap(); + CHECK_NE(wrap, nullptr); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(env, + wrap->get_async_id()); + req_wrap = WriteWrap::New(env, req_wrap_obj, this); + } err = DoWrite(req_wrap, bufs, count, nullptr); if (HasWriteQueue()) @@ -278,7 +282,6 @@ int StreamBase::WriteString(const FunctionCallbackInfo<Value>& args) { Local<Object> req_wrap_obj = args[0].As<Object>(); Local<String> string = args[1].As<String>(); Local<Object> send_handle_obj; - AsyncWrap* wrap; if (args[2]->IsObject()) send_handle_obj = args[2].As<Object>(); @@ -329,10 +332,13 @@ int StreamBase::WriteString(const FunctionCallbackInfo<Value>& args) { CHECK_EQ(count, 1); } - wrap = GetAsyncWrap(); - if (wrap != nullptr) - env->set_default_trigger_async_id(wrap->get_async_id()); - req_wrap = WriteWrap::New(env, req_wrap_obj, this, storage_size); + { + AsyncWrap* wrap = GetAsyncWrap(); + CHECK_NE(wrap, nullptr); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(env, + wrap->get_async_id()); + req_wrap = WriteWrap::New(env, req_wrap_obj, this, storage_size); + } data = req_wrap->Extra(); diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index 233e3a1a15..3a0a3f295e 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -56,7 +56,8 @@ Local<Object> TCPWrap::Instantiate(Environment* env, AsyncWrap* parent, TCPWrap::SocketType type) { EscapableHandleScope handle_scope(env->isolate()); - AsyncHooks::InitScope init_scope(env, parent->get_async_id()); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope( + env, parent->get_async_id()); CHECK_EQ(env->tcp_constructor_template().IsEmpty(), false); Local<Function> constructor = env->tcp_constructor_template()->GetFunction(); CHECK_EQ(constructor.IsEmpty(), false); @@ -292,7 +293,8 @@ void TCPWrap::Connect(const FunctionCallbackInfo<Value>& args) { int err = uv_ip4_addr(*ip_address, port, &addr); if (err == 0) { - env->set_default_trigger_async_id(wrap->get_async_id()); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope( + env, wrap->get_async_id()); ConnectWrap* req_wrap = new ConnectWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_TCPCONNECTWRAP); err = uv_tcp_connect(req_wrap->req(), @@ -328,7 +330,8 @@ void TCPWrap::Connect6(const FunctionCallbackInfo<Value>& args) { int err = uv_ip6_addr(*ip_address, port, &addr); if (err == 0) { - env->set_default_trigger_async_id(wrap->get_async_id()); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope( + env, wrap->get_async_id()); ConnectWrap* req_wrap = new ConnectWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_TCPCONNECTWRAP); err = uv_tcp_connect(req_wrap->req(), diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index 55e12e4278..27be93abd0 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -357,8 +357,12 @@ void UDPWrap::DoSend(const FunctionCallbackInfo<Value>& args, int family) { node::Utf8Value address(env->isolate(), args[4]); const bool have_callback = args[5]->IsTrue(); - env->set_default_trigger_async_id(wrap->get_async_id()); - SendWrap* req_wrap = new SendWrap(env, req_wrap_obj, have_callback); + SendWrap* req_wrap; + { + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope( + env, wrap->get_async_id()); + req_wrap = new SendWrap(env, req_wrap_obj, have_callback); + } size_t msg_size = 0; MaybeStackBuffer<uv_buf_t, 16> bufs(count); @@ -507,7 +511,9 @@ Local<Object> UDPWrap::Instantiate(Environment* env, AsyncWrap* parent, UDPWrap::SocketType type) { EscapableHandleScope scope(env->isolate()); - AsyncHooks::InitScope init_scope(env, parent->get_async_id()); + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope( + env, parent->get_async_id()); + // If this assert fires then Initialize hasn't been called yet. CHECK_EQ(env->udp_constructor_function().IsEmpty(), false); Local<Object> instance = env->udp_constructor_function() |