Fixed the race condition in NamedThread

Before this fix, `NamedThread::getCurrentThreadName()` might be called
from the new thread (that already started executing) before the
assignment to `thread_` in the `NamedThread` constructor had taken
place. Then the query to the thread id (`nt->thread_.get_id()`) on such
a thread did not return the correct id, and the end result was that a
new (numbered) thread name was synthesized for that thread. From that
point onward, concurrency orchestration via `log_debug()` (which relies
on correct thread names) broke, typically resulting in a deadlock.

This fix slightly reduces the generality of the `NamedThread` constructor
(support for arguments to the callable has been dropped) since it is not
currently needed (and even if it was needed, it could be circumvented
with an extra lambda expression).

Author: veloman-yunkan

src/namedthread.cpp

@@ -31,13 +31,14 @@ namespace zim
 namespace
 {
 
-std::mutex mutex_;
 size_t threadCounter_ = 0;
 std::vector<const NamedThread*> namedThreads_;
 std::map<std::thread::id, std::string> threadId2NameMap_;
 
 } // unnamed namespace
 
+std::mutex NamedThread::mutex_;
+
 NamedThread::NamedThread(const std::string& name)
   : name_(name)
 {

src/namedthread.h

@@ -20,6 +20,7 @@
 #ifndef OPENZIM_LIBZIM_NAMEDTHREAD_H
 #define OPENZIM_LIBZIM_NAMEDTHREAD_H
 
+#include <mutex>
 #include <string>
 #include <thread>
 
@@ -34,11 +35,17 @@ class LIBZIM_PRIVATE_API NamedThread
   explicit NamedThread(const std::string& name);
 
 public:
-  template <class F, class... Args>
-  NamedThread(const std::string& name, F&& f, Args&&... args)
+  template <class F>
+  NamedThread(const std::string& name, F&& f)
     : NamedThread(name)
   {
-    thread_ = std::thread(std::forward<F>(f), std::forward<Args>(args)...);
+    // Ensure that f starts executing after the assignment to
+    // the thread_ data member has completed (so that any possible
+    // calls to NamedThread::getCurrentThreadName() from inside f()
+    // read the correct value of thread id).
+    std::lock_guard<std::mutex> lock(mutex_);
+
+    thread_ = std::thread([f]() { mutex_.lock(); mutex_.unlock(); f(); });
   }
 
   ~NamedThread();
@@ -53,6 +60,8 @@ class LIBZIM_PRIVATE_API NamedThread
 private:
   const std::string name_;
   std::thread thread_;
+
+  static std::mutex mutex_;
 };
 
 } // namespace zim