diff options
author | Ben Noordhuis <info@bnoordhuis.nl> | 2020-02-13 18:31:05 +0300 |
---|---|---|
committer | Shelley Vohr <shelley.vohr@gmail.com> | 2020-02-17 23:40:55 +0300 |
commit | 1dec9d196f3d382b76b2611cb9d0c3994ced6157 (patch) | |
tree | 119cd1673d48a5804fa78b910613f4f51cc7315f /src/inspector_io.cc | |
parent | e23023d6856168074efe64dc769303f169244c4f (diff) |
src: wrap HostPort in ExclusiveAccess
I found it exceedingly hard to figure out if there is a race condition
where one thread reads the inspector agent's HostPort's properties while
another modifies them concurrently.
I think the answer is "no, there isn't" but with this commit use sites
are forced to unwrap the object (and acquire the mutex in the process),
making it a great deal easier to reason about correctness.
PR-URL: https://github.com/nodejs/node/pull/31717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: David Carlier <devnexen@gmail.com>
Diffstat (limited to 'src/inspector_io.cc')
-rw-r--r-- | src/inspector_io.cc | 21 |
1 files changed, 15 insertions, 6 deletions
diff --git a/src/inspector_io.cc b/src/inspector_io.cc index ed9035136c5..75290317d2f 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> InspectorIo::Start( std::shared_ptr<MainThreadHandle> main_thread, const std::string& path, - std::shared_ptr<HostPort> host_port, + std::shared_ptr<ExclusiveAccess<HostPort>> host_port, const InspectPublishUid& inspect_publish_uid) { auto io = std::unique_ptr<InspectorIo>( new InspectorIo(main_thread, @@ -254,7 +254,7 @@ std::unique_ptr<InspectorIo> InspectorIo::Start( InspectorIo::InspectorIo(std::shared_ptr<MainThreadHandle> main_thread, const std::string& path, - std::shared_ptr<HostPort> host_port, + std::shared_ptr<ExclusiveAccess<HostPort>> 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<InspectorIoDelegate> delegate( new InspectorIoDelegate(queue, main_thread_, id_, script_path, script_name_)); + std::string host; + int port; + { + ExclusiveAccess<HostPort>::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<HostPort>::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<HostPort>::Scoped host_port(host_port_); + return FormatWsAddress(host_port->host(), host_port->port(), id_, true); } InspectorIoDelegate::InspectorIoDelegate( |