server: Made connections in a server have a mutex to avoid use after frees

2 files changed, 29 insertions, 7 deletions

diff --git a/Source/cmServer.cxx b/Source/cmServer.cxx
index 6a63797..c7f8704 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<cmConnection>& 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 cb7df10..d8f73c1 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<std::unique_ptr<cmConnection>> Connections;
 
   bool ServeThreadRunning = false;