Fix thread safety issue

5 files changed, 123 insertions, 34 deletions

diff --git a/butl/config b/butl/config
new file mode 100644
index 0000000..71ebd23
--- /dev/null
+++ b/butl/config
@@ -0,0 +1,47 @@
+// file      : butl/config -*- C++ -*-
+// copyright : Copyright (c) 2014-2016 Code Synthesis Ltd
+// license   : MIT; see accompanying LICENSE file
+
+#ifndef BUTL_CONFIG
+#define BUTL_CONFIG
+
+#include <cstddef> // _LIBCPP_VERSION, __GLIBCXX__
+
+// thread_local
+//
+// If this macro is undefined then we assume one can use __thread but only
+// for values that do not require dynamic (runtime) initialization.
+//
+// Apparently Apple's Clang "temporarily disabled" C++11 thread_local
+// until they can implement a "fast" version, which reportedly happened in
+// XCode 8. So for now we will continue using __thread for this target.
+//
+#ifdef __apple_build_version__
+#  if __apple_build_version__ >= 8000000
+#    define BUTL_CXX11_THREAD_LOCAL
+#  endif
+#else
+#    define BUTL_CXX11_THREAD_LOCAL
+#endif
+
+// std::uncaught_exceptions()
+//
+// VC has it since 1900 which is the lowest version we support.
+//
+#if defined(_MSC_VER)
+#  define BUTL_CXX17_UNCAUGHT_EXCEPTIONS
+//
+// Clang's libc++ seems to have it for a while so we assume it's there (it
+// doesn't define a feature test macros).
+//
+#elif defined(_LIBCPP_VERSION)
+#  define BUTL_CXX17_UNCAUGHT_EXCEPTIONS
+//
+// GCC libstdc++ has it since GCC 6 and it defines the feature test macro.
+// We will also use this for any other runtime.
+//
+#elif defined(__cpp_lib_uncaught_exceptions)
+#  define BUTL_CXX17_UNCAUGHT_EXCEPTIONS
+#endif
+
+#endif // BUTL_CONFIG
diff --git a/butl/diagnostics b/butl/diagnostics
index b56ef10..b76e384 100644
--- a/butl/diagnostics
+++ b/butl/diagnostics
@@ -9,7 +9,7 @@
 #include <ostream>
 #include <sstream>
 #include <utility>   // move(), forward()
-#include <exception> // uncaught_exception()
+#include <exception> // uncaught_exception(s)()
 
 #include <butl/export>
 #include <butl/utility>
@@ -36,17 +36,21 @@ namespace butl
       return r;
     }
 
-    diag_record (): empty_ (true), epilogue_ (nullptr) {}
+    diag_record ()
+        :
+#ifdef BUTL_CXX17_UNCAUGHT_EXCEPTIONS
+        uncaught_ (std::uncaught_exceptions ()),
+#endif
+        empty_ (true),
+        epilogue_ (nullptr) {}
 
     template <typename B>
     explicit
-    diag_record (const diag_prologue<B>& p)
-        : empty_ (true), epilogue_ (nullptr) { *this << p;}
+    diag_record (const diag_prologue<B>& p): diag_record () { *this << p;}
 
     template <typename B>
     explicit
-    diag_record (const diag_mark<B>& m)
-        : empty_ (true), epilogue_ (nullptr) { *this << m;}
+    diag_record (const diag_mark<B>& m): diag_record () { *this << m;}
 
     ~diag_record () noexcept (false);
 
@@ -85,19 +89,16 @@ namespace butl
     diag_record (diag_record&&);
 #else
     diag_record (diag_record&& r)
-#ifndef __GLIBCXX__
-        : os (std::move (r.os))
+        :
+#ifdef BUTL_CXX17_UNCAUGHT_EXCEPTIONS
+        uncaught_ (r.uncaught_),
 #endif
+        empty_ (r.empty_),
+        epilogue_ (r.epilogue_),
+        os (std::move (r.os))
     {
-      empty_ = r.empty_;
-      epilogue_ = r.epilogue_;
-
       if (!empty_)
       {
-#ifdef __GLIBCXX__
-        // os << r.os.str ();
-        assert (false); // No way to copy extra stream data?
-#endif
         r.empty_ = true;
         r.epilogue_ = nullptr;
       }
@@ -109,12 +110,15 @@ namespace butl
     diag_record (const diag_record&) = delete;
     diag_record& operator= (const diag_record&) = delete;
 
-  public:
-    mutable std::ostringstream os;
-
   protected:
+#ifdef BUTL_CXX17_UNCAUGHT_EXCEPTIONS
+    const int uncaught_;
+#endif
     mutable bool empty_;
     mutable diag_epilogue* epilogue_;
+
+  public:
+    mutable std::ostringstream os;
   };
 
   template <typename B>
diff --git a/butl/diagnostics.cxx b/butl/diagnostics.cxx
index e4c22dc..ad84810 100644
--- a/butl/diagnostics.cxx
+++ b/butl/diagnostics.cxx
@@ -28,14 +28,19 @@ namespace butl
   diag_record::
   ~diag_record () noexcept (false)
   {
-    // Don't flush the record if this destructor was called as part of
-    // the stack unwinding. Right now this means we cannot use this
-    // mechanism in destructors, which is not a big deal, except for
-    // one place: exception_guard. So for now we are going to have
-    // this ugly special check which we will be able to get rid of
-    // once C++17 uncaught_exceptions() becomes available.
+    // Don't flush the record if this destructor was called as part of the
+    // stack unwinding.
+    //
+#ifdef BUTL_CXX17_UNCAUGHT_EXCEPTIONS
+    if (uncaught_ == std::uncaught_exceptions ())
+      flush ();
+#else
+    // Fallback implementation. Right now this means we cannot use this
+    // mechanism in destructors, which is not a big deal, except for one
+    // place: exception_guard. Thus the ugly special check.
     //
     if (!std::uncaught_exception () || exception_unwinding_dtor)
       flush ();
+#endif
   }
 }
diff --git a/butl/utility b/butl/utility
index c57aabb..02d9331 100644
--- a/butl/utility
+++ b/butl/utility
@@ -9,10 +9,11 @@
 #include <cstddef>      // size_t
 #include <utility>      // move(), forward()
 #include <cstring>      // strcmp(), strlen()
-#include <exception>    // uncaught_exception()
+#include <exception>    // uncaught_exception(s)()
 //#include <functional> // hash
 
 #include <butl/export>
+#include <butl/config>
 
 namespace butl
 {
@@ -153,15 +154,6 @@ namespace butl
   // Call a function if there is an exception.
   //
 
-  // True means we are in the body of a destructor that is being called as
-  // part of the exception stack unwindining. Used to compensate for the
-  // deficiencies of uncaught_exception() until C++17 uncaught_exceptions()
-  // becomes available.
-  //
-  // @@ MT: will have to be TLS.
-  //
-  LIBBUTL_EXPORT extern bool exception_unwinding_dtor;
-
   template <typename F>
   struct exception_guard;
 
@@ -172,6 +164,39 @@ namespace butl
     return exception_guard<F> (std::move (f));
   }
 
+#ifdef BUTL_CXX17_UNCAUGHT_EXCEPTIONS
+  template <typename F>
+  struct exception_guard
+  {
+    exception_guard (F f)
+        : f_ (std::move (f)),
+          u_ (std::uncaught_exceptions ()) {}
+
+    ~exception_guard ()
+    {
+      if (u_ != std::uncaught_exceptions ())
+        f_ ();
+    }
+
+  private:
+    F f_;
+    int u_;
+  };
+#else
+  // Fallback implementation using a TLS flag.
+  //
+  // True means we are in the body of a destructor that is being called as
+  // part of the exception stack unwindining.
+  //
+  LIBBUTL_EXPORT
+  extern
+#ifdef BUTL_CXX11_THREAD_LOCAL
+  thread_local
+#else
+  __thread
+#endif
+  bool exception_unwinding_dtor;
+
   template <typename F>
   struct exception_guard
   {
@@ -189,6 +214,7 @@ namespace butl
   private:
     F f_;
   };
+#endif
 }
 
 #include <butl/utility.ixx>
diff --git a/butl/utility.cxx b/butl/utility.cxx
index d6f418c..032d178 100644
--- a/butl/utility.cxx
+++ b/butl/utility.cxx
@@ -6,5 +6,12 @@
 
 namespace butl
 {
+#ifndef BUTL_CXX17_UNCAUGHT_EXCEPTIONS
+#ifdef BUTL_CXX11_THREAD_LOCAL
+  thread_local
+#else
+  __thread
+#endif
   bool exception_unwinding_dtor = false;
+#endif
 }