From a4199b808fd678f74935d540490eae9dc78a9ffe Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 13 Dec 2016 16:39:58 +0200 Subject: Fix thread safety issue --- butl/config | 47 +++++++++++++++++++++++++++++++++++++++++++++++ butl/diagnostics | 40 ++++++++++++++++++++++------------------ butl/diagnostics.cxx | 17 +++++++++++------ butl/utility | 46 ++++++++++++++++++++++++++++++++++++---------- butl/utility.cxx | 7 +++++++ 5 files changed, 123 insertions(+), 34 deletions(-) create mode 100644 butl/config 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 // _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 #include #include // move(), forward() -#include // uncaught_exception() +#include // uncaught_exception(s)() #include #include @@ -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 explicit - diag_record (const diag_prologue& p) - : empty_ (true), epilogue_ (nullptr) { *this << p;} + diag_record (const diag_prologue& p): diag_record () { *this << p;} template explicit - diag_record (const diag_mark& m) - : empty_ (true), epilogue_ (nullptr) { *this << m;} + diag_record (const diag_mark& 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 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 // size_t #include // move(), forward() #include // strcmp(), strlen() -#include // uncaught_exception() +#include // uncaught_exception(s)() //#include // hash #include +#include 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 struct exception_guard; @@ -172,6 +164,39 @@ namespace butl return exception_guard (std::move (f)); } +#ifdef BUTL_CXX17_UNCAUGHT_EXCEPTIONS + template + 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 struct exception_guard { @@ -189,6 +214,7 @@ namespace butl private: F f_; }; +#endif } #include 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 } -- cgit v1.1