From 2c9667237d0314cadb8f85d45777089aac6833a4 Mon Sep 17 00:00:00 2001
From: Martin Pecka <peci1@seznam.cz>
Date: Wed, 2 Sep 2020 04:09:17 +0200
Subject: [PATCH] Use random name for manager semaphore

Fixes #56.

Signed-off-by: Martin Pecka <peckama2@fel.cvut.cz>
---
 src/CMakeLists.txt |  1 +
 src/Manager.cc     | 35 ++++++++++++++++++++++++++++-------
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 6e6e7672..d4fa818e 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -21,6 +21,7 @@ target_link_libraries(${PROJECT_LIBRARY_TARGET_NAME}
     TINYXML2::TINYXML2
   PRIVATE
     ignition-common${IGN_COMMON_MAJOR_VER}::ignition-common${IGN_COMMON_MAJOR_VER}
+    ignition-math${IGN_MATH_MAJOR_VER}::ignition-math${IGN_MATH_MAJOR_VER}
     ${BACKWARD_LIBRARIES}
 )
 
diff --git a/src/Manager.cc b/src/Manager.cc
index 36451f93..e9a4d51a 100644
--- a/src/Manager.cc
+++ b/src/Manager.cc
@@ -24,10 +24,12 @@
 #include <unistd.h>
 
 #include <condition_variable>
+#include <ctime>
 #include <list>
 #include <mutex>
 #include <numeric>
 #include <queue>
+#include <random>
 #include <string>
 #include <thread>
 #include <unordered_set>
@@ -36,6 +38,7 @@
 #include <ignition/common/Console.hh>
 #include <ignition/common/SignalHandler.hh>
 #include <ignition/common/SystemPaths.hh>
+#include <ignition/math/Rand.hh>
 #include <ignition/plugin/Loader.hh>
 
 #include "ignition/launch/config.hh"
@@ -47,8 +50,6 @@
 using namespace ignition::launch;
 using namespace std::chrono_literals;
 
-static constexpr const char* kSemaphoreName = "/child_semaphore";
-
 /// \brief A class to encapsulate an executable (program) to run.
 class Executable
 {
@@ -177,6 +178,9 @@ class ignition::launch::ManagerPrivate
 
   /// \brief Semaphore to prevent restartThread from being a spinlock
   private: sem_t *stoppedChildSem;
+  
+  /// \brief Name of the semaphore created by stoppedChildSem.
+  private: std::string stoppedChildSemName;
 
   /// \brief Thread containing the restart loop
   private: std::thread restartThread;
@@ -294,10 +298,25 @@ ManagerPrivate::ManagerPrivate()
   this->sigHandler->AddCallback(
       std::bind(&ManagerPrivate::OnSigIntTerm, this, std::placeholders::_1));
 
+  {
+    // The semaphore we initialize in the next section needs a unique name, so we need
+    // to seed a random number generator with a quality random number. Especially time(0)
+    // itself is not a good seed as there may be multiple processes launched at the same time.
+    const auto time_seed = static_cast<size_t>(std::time(nullptr));
+    const auto pid_seed = std::hash<std::thread::id>()(std::this_thread::get_id());
+    std::seed_seq seed_value{time_seed, pid_seed};
+    std::vector<size_t> seeds(1);
+    seed_value.generate(seeds.begin(), seeds.end());
+    math::Rand::Seed(seeds[0]);
+  }
+  const auto semRandomId = math::Rand::IntUniform(0, std::numeric_limits<int32_t>::max());
+
   // Initialize semaphore
-  this->stoppedChildSem = sem_open(kSemaphoreName, O_CREAT, 0644, 1);
+  this->stoppedChildSemName = std::string("ign-launch-") + std::to_string(semRandomId);
+  this->stoppedChildSem = sem_open(this->stoppedChildSemName.c_str(), O_CREAT, 0644, 1);
   if (this->stoppedChildSem == SEM_FAILED) {
-    ignerr << "Error initializing semaphore: " << strerror(errno) << std::endl;
+    ignerr << "Error initializing semaphore " << this->stoppedChildSemName << ": " <<
+      strerror(errno) << std::endl;
   }
 
   // Register a signal handler to capture child process death events.
@@ -329,12 +348,14 @@ ManagerPrivate::~ManagerPrivate()
   {
     if (sem_close(this->stoppedChildSem) == -1)
     {
-      ignerr << "Failed to close semaphore: " << strerror(errno) << std::endl;
+      ignerr << "Failed to close semaphore " << this->stoppedChildSemName << ": " <<
+        strerror(errno) << std::endl;
     }
 
-    if (sem_unlink(kSemaphoreName) == -1)
+    if (sem_unlink(this->stoppedChildSemName.c_str()) == -1)
     {
-      ignerr << "Failed to unlink semaphore: " << strerror(errno) << std::endl;
+      ignerr << "Failed to unlink semaphore " << this->stoppedChildSemName << ": " <<
+        strerror(errno) << std::endl;
     }
   }
 }