From 882dcef8e02e432e6469b7fca38aff212d1541ab Mon Sep 17 00:00:00 2001 From: Justin Berger Date: Wed, 19 Jul 2017 22:04:13 -0600 Subject: [PATCH] server: Made connections in a server have a mutex to avoid use after frees --- Source/cmServer.cxx | 35 ++++++++++++++++++++++++++++------- Source/cmServer.h | 1 + 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/Source/cmServer.cxx b/Source/cmServer.cxx index 6a63797067..c7f870444d 100644 --- a/Source/cmServer.cxx +++ b/Source/cmServer.cxx @@ -244,9 +244,11 @@ cmFileMonitor* cmServer::FileMonitor() const void cmServer::WriteJsonObject(const Json::Value& jsonValue, const DebugInfo* debug) const { + uv_rwlock_rdlock(&ConnectionsMutex); for (auto& connection : this->Connections) { WriteJsonObject(connection.get(), jsonValue, debug); } + uv_rwlock_rdunlock(&ConnectionsMutex); } void cmServer::WriteJsonObject(cmConnection* connection, @@ -443,10 +445,15 @@ bool cmServerBase::Serve(std::string* errorMessage) OnServeStart(); - for (auto& connection : Connections) { - if (!connection->OnServeStart(errorMessage)) { - return false; + { + uv_rwlock_rdlock(&ConnectionsMutex); + for (auto& connection : Connections) { + if (!connection->OnServeStart(errorMessage)) { + uv_rwlock_rdunlock(&ConnectionsMutex); + return false; + } } + uv_rwlock_rdunlock(&ConnectionsMutex); } if (uv_run(&Loop, UV_RUN_DEFAULT) != 0) { @@ -479,10 +486,14 @@ void cmServerBase::StartShutDown() uv_signal_stop(&this->SIGHUPHandler); } - for (auto& connection : Connections) { - connection->OnConnectionShuttingDown(); + { + uv_rwlock_wrlock(&ConnectionsMutex); + for (auto& connection : Connections) { + connection->OnConnectionShuttingDown(); + } + Connections.clear(); + uv_rwlock_wrunlock(&ConnectionsMutex); } - Connections.clear(); uv_walk(&Loop, on_walk_to_shutdown, nullptr); } @@ -496,7 +507,12 @@ bool cmServerBase::OnSignal(int signum) cmServerBase::cmServerBase(cmConnection* connection) { - uv_loop_init(&Loop); + auto err = uv_loop_init(&Loop); + (void)err; + assert(err == 0); + + err = uv_rwlock_init(&ConnectionsMutex); + assert(err == 0); AddNewConnection(connection); } @@ -510,11 +526,14 @@ cmServerBase::~cmServerBase() } uv_loop_close(&Loop); + uv_rwlock_destroy(&ConnectionsMutex); } void cmServerBase::AddNewConnection(cmConnection* ownedConnection) { + uv_rwlock_wrlock(&ConnectionsMutex); Connections.emplace_back(ownedConnection); + uv_rwlock_wrunlock(&ConnectionsMutex); ownedConnection->SetServer(this); } @@ -528,9 +547,11 @@ void cmServerBase::OnDisconnect(cmConnection* pConnection) auto pred = [pConnection](const std::unique_ptr& m) { return m.get() == pConnection; }; + uv_rwlock_wrlock(&ConnectionsMutex); Connections.erase( std::remove_if(Connections.begin(), Connections.end(), pred), Connections.end()); + uv_rwlock_wrunlock(&ConnectionsMutex); if (Connections.empty()) { StartShutDown(); } diff --git a/Source/cmServer.h b/Source/cmServer.h index cb7df1093a..d8f73c11e5 100644 --- a/Source/cmServer.h +++ b/Source/cmServer.h @@ -61,6 +61,7 @@ public: void OnDisconnect(cmConnection* pConnection); protected: + mutable uv_rwlock_t ConnectionsMutex; std::vector> Connections; bool ServeThreadRunning = false;