diff --git a/src/env-inl.h b/src/env-inl.h index 71f0ff7f6d74fa..7f29b93cc12a97 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -743,7 +743,8 @@ inline uint64_t Environment::heap_prof_interval() const { #endif // HAVE_INSPECTOR -inline std::shared_ptr Environment::inspector_host_port() { +inline +std::shared_ptr> Environment::inspector_host_port() { return inspector_host_port_; } diff --git a/src/env.cc b/src/env.cc index 36b41c5fac56c4..e54e367f0be579 100644 --- a/src/env.cc +++ b/src/env.cc @@ -321,7 +321,8 @@ Environment::Environment(IsolateData* isolate_data, // part of the per-Isolate option set, for which in turn the defaults are // part of the per-process option set. options_.reset(new EnvironmentOptions(*isolate_data->options()->per_env)); - inspector_host_port_.reset(new HostPort(options_->debug_options().host_port)); + inspector_host_port_.reset( + new ExclusiveAccess(options_->debug_options().host_port)); #if HAVE_INSPECTOR // We can only create the inspector agent after having cloned the options. diff --git a/src/env.h b/src/env.h index e100802ee74cc9..3b577e40307640 100644 --- a/src/env.h +++ b/src/env.h @@ -1227,7 +1227,7 @@ class Environment : public MemoryRetainer { void* data); inline std::shared_ptr options(); - inline std::shared_ptr inspector_host_port(); + inline std::shared_ptr> inspector_host_port(); // The BaseObject count is a debugging helper that makes sure that there are // no memory leaks caused by BaseObjects staying alive longer than expected @@ -1326,7 +1326,7 @@ class Environment : public MemoryRetainer { // server starts listening), but when the inspector server starts // the inspector_host_port_->port() will be the actual port being // used. - std::shared_ptr inspector_host_port_; + std::shared_ptr> inspector_host_port_; std::vector exec_argv_; std::vector argv_; std::string exec_path_; diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index fe60c82cb5590d..b7ccf641c299f5 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -757,7 +757,7 @@ Agent::~Agent() {} bool Agent::Start(const std::string& path, const DebugOptions& options, - std::shared_ptr host_port, + std::shared_ptr> host_port, bool is_main) { path_ = path; debug_options_ = options; diff --git a/src/inspector_agent.h b/src/inspector_agent.h index d5088a1b546904..089077370db049 100644 --- a/src/inspector_agent.h +++ b/src/inspector_agent.h @@ -48,7 +48,7 @@ class Agent { // Create client_, may create io_ if option enabled bool Start(const std::string& path, const DebugOptions& options, - std::shared_ptr host_port, + std::shared_ptr> host_port, bool is_main); // Stop and destroy io_ void Stop(); @@ -110,7 +110,7 @@ class Agent { void RequestIoThreadStart(); const DebugOptions& options() { return debug_options_; } - std::shared_ptr host_port() { return host_port_; } + std::shared_ptr> host_port() { return host_port_; } void ContextCreated(v8::Local context, const ContextInfo& info); // Interface for interacting with inspectors in worker threads @@ -133,7 +133,7 @@ class Agent { // pointer which is meant to store the actual host and port of the inspector // server. DebugOptions debug_options_; - std::shared_ptr host_port_; + std::shared_ptr> host_port_; bool pending_enable_async_hook_ = false; bool pending_disable_async_hook_ = false; diff --git a/src/inspector_io.cc b/src/inspector_io.cc index ed9035136c51db..75290317d2fcae 100644 --- a/src/inspector_io.cc +++ b/src/inspector_io.cc @@ -239,7 +239,7 @@ class InspectorIoDelegate: public node::inspector::SocketServerDelegate { std::unique_ptr InspectorIo::Start( std::shared_ptr main_thread, const std::string& path, - std::shared_ptr host_port, + std::shared_ptr> host_port, const InspectPublishUid& inspect_publish_uid) { auto io = std::unique_ptr( new InspectorIo(main_thread, @@ -254,7 +254,7 @@ std::unique_ptr InspectorIo::Start( InspectorIo::InspectorIo(std::shared_ptr main_thread, const std::string& path, - std::shared_ptr host_port, + std::shared_ptr> host_port, const InspectPublishUid& inspect_publish_uid) : main_thread_(main_thread), host_port_(host_port), @@ -293,10 +293,17 @@ void InspectorIo::ThreadMain() { std::unique_ptr delegate( new InspectorIoDelegate(queue, main_thread_, id_, script_path, script_name_)); + std::string host; + int port; + { + ExclusiveAccess::Scoped host_port(host_port_); + host = host_port->host(); + port = host_port->port(); + } InspectorSocketServer server(std::move(delegate), &loop, - host_port_->host(), - host_port_->port(), + std::move(host), + port, inspect_publish_uid_); request_queue_ = queue->handle(); // Its lifetime is now that of the server delegate @@ -304,7 +311,8 @@ void InspectorIo::ThreadMain() { { Mutex::ScopedLock scoped_lock(thread_start_lock_); if (server.Start()) { - host_port_->set_port(server.Port()); + ExclusiveAccess::Scoped host_port(host_port_); + host_port->set_port(server.Port()); } thread_start_condition_.Broadcast(scoped_lock); } @@ -313,7 +321,8 @@ void InspectorIo::ThreadMain() { } std::string InspectorIo::GetWsUrl() const { - return FormatWsAddress(host_port_->host(), host_port_->port(), id_, true); + ExclusiveAccess::Scoped host_port(host_port_); + return FormatWsAddress(host_port->host(), host_port->port(), id_, true); } InspectorIoDelegate::InspectorIoDelegate( diff --git a/src/inspector_io.h b/src/inspector_io.h index e9f94056733994..10cca26ff1b9ee 100644 --- a/src/inspector_io.h +++ b/src/inspector_io.h @@ -30,7 +30,7 @@ class InspectorIo { static std::unique_ptr Start( std::shared_ptr main_thread, const std::string& path, - std::shared_ptr host_port, + std::shared_ptr> host_port, const InspectPublishUid& inspect_publish_uid); // Will block till the transport thread shuts down @@ -42,7 +42,7 @@ class InspectorIo { private: InspectorIo(std::shared_ptr handle, const std::string& path, - std::shared_ptr host_port, + std::shared_ptr> host_port, const InspectPublishUid& inspect_publish_uid); // Wrapper for agent->ThreadMain() @@ -57,7 +57,7 @@ class InspectorIo { // Used to post on a frontend interface thread, lives while the server is // running std::shared_ptr request_queue_; - std::shared_ptr host_port_; + std::shared_ptr> host_port_; InspectPublishUid inspect_publish_uid_; // The IO thread runs its own uv_loop to implement the TCP server off diff --git a/src/inspector_js_api.cc b/src/inspector_js_api.cc index 45b1c83bea7f34..703c9ff598fdf2 100644 --- a/src/inspector_js_api.cc +++ b/src/inspector_js_api.cc @@ -278,12 +278,14 @@ void Open(const FunctionCallbackInfo& args) { if (args.Length() > 0 && args[0]->IsUint32()) { uint32_t port = args[0].As()->Value(); - agent->host_port()->set_port(static_cast(port)); + ExclusiveAccess::Scoped host_port(agent->host_port()); + host_port->set_port(static_cast(port)); } if (args.Length() > 1 && args[1]->IsString()) { Utf8Value host(env->isolate(), args[1].As()); - agent->host_port()->set_host(*host); + ExclusiveAccess::Scoped host_port(agent->host_port()); + host_port->set_host(*host); } agent->StartIoThread(); diff --git a/src/node_mutex.h b/src/node_mutex.h index 807e1444f1eb8b..b82505cc639b7a 100644 --- a/src/node_mutex.h +++ b/src/node_mutex.h @@ -6,6 +6,9 @@ #include "util.h" #include "uv.h" +#include // std::shared_ptr +#include // std::forward + namespace node { template class ConditionVariableBase; @@ -15,6 +18,51 @@ struct LibuvMutexTraits; using ConditionVariable = ConditionVariableBase; using Mutex = MutexBase; +template +class ExclusiveAccess { + public: + ExclusiveAccess() = default; + + template + explicit ExclusiveAccess(Args&&... args) + : item_(std::forward(args)...) {} + + ExclusiveAccess(const ExclusiveAccess&) = delete; + ExclusiveAccess& operator=(const ExclusiveAccess&) = delete; + + class Scoped { + public: + // ExclusiveAccess will commonly be used in conjuction with std::shared_ptr + // and without this constructor it's too easy to forget to keep a reference + // around to the shared_ptr while operating on the ExclusiveAccess object. + explicit Scoped(const std::shared_ptr& shared) + : shared_(shared) + , scoped_lock_(shared->mutex_) + , pointer_(&shared->item_) {} + + explicit Scoped(ExclusiveAccess* exclusive_access) + : shared_() + , scoped_lock_(exclusive_access->mutex_) + , pointer_(&exclusive_access->item_) {} + + T& operator*() const { return *pointer_; } + T* operator->() const { return pointer_; } + + Scoped(const Scoped&) = delete; + Scoped& operator=(const Scoped&) = delete; + + private: + std::shared_ptr shared_; + typename MutexT::ScopedLock scoped_lock_; + T* const pointer_; + }; + + private: + friend class ScopedLock; + MutexT mutex_; + T item_; +}; + template class MutexBase { public: diff --git a/src/node_process_object.cc b/src/node_process_object.cc index 7cf8c125009606..ddbb58abe535b0 100644 --- a/src/node_process_object.cc +++ b/src/node_process_object.cc @@ -51,7 +51,8 @@ static void ProcessTitleSetter(Local property, static void DebugPortGetter(Local property, const PropertyCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); - int port = env->inspector_host_port()->port(); + ExclusiveAccess::Scoped host_port(env->inspector_host_port()); + int port = host_port->port(); info.GetReturnValue().Set(port); } @@ -60,7 +61,8 @@ static void DebugPortSetter(Local property, const PropertyCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); int32_t port = value->Int32Value(env->context()).FromMaybe(0); - env->inspector_host_port()->set_port(static_cast(port)); + ExclusiveAccess::Scoped host_port(env->inspector_host_port()); + host_port->set_port(static_cast(port)); } static void GetParentProcessId(Local property,