From 997fe7f9c93181639bca69d212265b1ecb488103 Mon Sep 17 00:00:00 2001 From: Marco Wang Date: Thu, 7 Nov 2019 00:12:17 +0800 Subject: [PATCH 1/4] fix race conditions in LOG_EVERY_N * Use CMakeLists.txt to check the availability of std::atomic<> * Declare the static integer counters as std::atomic instead of int if C++11's atomic is available --- CMakeLists.txt | 6 ++++++ src/config.h.cmake.in | 3 +++ src/glog/logging.h.in | 44 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4f64b411e..9fd512142 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -213,6 +213,12 @@ std::aligned_storage::type data; int main() { } " HAVE_ALIGNED_STORAGE) +check_cxx_source_compiles (" +#include +std::atomic i; +int main() { } +" HAVE_CXX11_ATOMIC) + if (WITH_TLS) # Cygwin does not support the thread attribute. Don't bother. if (HAVE_GCC_TLS) diff --git a/src/config.h.cmake.in b/src/config.h.cmake.in index ee8cdfe22..3b35f5a40 100644 --- a/src/config.h.cmake.in +++ b/src/config.h.cmake.in @@ -192,6 +192,9 @@ /* Check whether aligned_storage and alignof present */ #cmakedefine HAVE_ALIGNED_STORAGE ${HAVE_ALIGNED_STORAGE} +/* Check whether C++11 atomic is available */ +#cmakedefine HAVE_CXX11_ATOMIC ${HAVE_CXX11_ATOMIC} + /* Version number of package */ #cmakedefine VERSION diff --git a/src/glog/logging.h.in b/src/glog/logging.h.in index 0ec46a167..f897b2416 100644 --- a/src/glog/logging.h.in +++ b/src/glog/logging.h.in @@ -86,6 +86,10 @@ #include #endif +#if HAVE_CXX11_ATOMIC +#include +#endif + @ac_google_start_namespace@ #if @ac_cv_have_uint16_t@ // the C99 format @@ -896,6 +900,45 @@ PLOG_IF(FATAL, GOOGLE_PREDICT_BRANCH_NOT_TAKEN((invocation) == -1)) \ #define LOG_OCCURRENCES LOG_EVERY_N_VARNAME(occurrences_, __LINE__) #define LOG_OCCURRENCES_MOD_N LOG_EVERY_N_VARNAME(occurrences_mod_n_, __LINE__) +#ifdef HAVE_CXX11_ATOMIC +#define SOME_KIND_OF_LOG_EVERY_N(severity, n, what_to_do) \ + static std::atomic LOG_OCCURRENCES(0), LOG_OCCURRENCES_MOD_N(0); \ + ++LOG_OCCURRENCES; \ + if (++LOG_OCCURRENCES_MOD_N > n) LOG_OCCURRENCES_MOD_N -= n; \ + if (LOG_OCCURRENCES_MOD_N == 1) \ + @ac_google_namespace@::LogMessage( \ + __FILE__, __LINE__, @ac_google_namespace@::GLOG_ ## severity, LOG_OCCURRENCES, \ + &what_to_do).stream() + +#define SOME_KIND_OF_LOG_IF_EVERY_N(severity, condition, n, what_to_do) \ + static std::atomic LOG_OCCURRENCES(0), LOG_OCCURRENCES_MOD_N(0); \ + ++LOG_OCCURRENCES; \ + if (condition && \ + ((LOG_OCCURRENCES_MOD_N=(LOG_OCCURRENCES_MOD_N + 1) % n) == (1 % n))) \ + @ac_google_namespace@::LogMessage( \ + __FILE__, __LINE__, @ac_google_namespace@::GLOG_ ## severity, LOG_OCCURRENCES, \ + &what_to_do).stream() + +#define SOME_KIND_OF_PLOG_EVERY_N(severity, n, what_to_do) \ + static std::atomic LOG_OCCURRENCES(0), LOG_OCCURRENCES_MOD_N(0); \ + ++LOG_OCCURRENCES; \ + if (++LOG_OCCURRENCES_MOD_N > n) LOG_OCCURRENCES_MOD_N -= n; \ + if (LOG_OCCURRENCES_MOD_N == 1) \ + @ac_google_namespace@::ErrnoLogMessage( \ + __FILE__, __LINE__, @ac_google_namespace@::GLOG_ ## severity, LOG_OCCURRENCES, \ + &what_to_do).stream() + +#define SOME_KIND_OF_LOG_FIRST_N(severity, n, what_to_do) \ + static std::atomic LOG_OCCURRENCES(0); \ + if (LOG_OCCURRENCES <= n) \ + ++LOG_OCCURRENCES; \ + if (LOG_OCCURRENCES <= n) \ + @ac_google_namespace@::LogMessage( \ + __FILE__, __LINE__, @ac_google_namespace@::GLOG_ ## severity, LOG_OCCURRENCES, \ + &what_to_do).stream() + +#else + #define SOME_KIND_OF_LOG_EVERY_N(severity, n, what_to_do) \ static int LOG_OCCURRENCES = 0, LOG_OCCURRENCES_MOD_N = 0; \ ++LOG_OCCURRENCES; \ @@ -931,6 +974,7 @@ PLOG_IF(FATAL, GOOGLE_PREDICT_BRANCH_NOT_TAKEN((invocation) == -1)) \ @ac_google_namespace@::LogMessage( \ __FILE__, __LINE__, @ac_google_namespace@::GLOG_ ## severity, LOG_OCCURRENCES, \ &what_to_do).stream() +#endif namespace glog_internal_namespace_ { template From 46b536522844cd0cd18bcb384664cc94dd0b21c5 Mon Sep 17 00:00:00 2001 From: Marco Wang Date: Thu, 7 Nov 2019 01:09:25 +0800 Subject: [PATCH 2/4] implement fallback mechanism for pre C++11 compilers --- src/glog/logging.h.in | 58 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 51 insertions(+), 7 deletions(-) diff --git a/src/glog/logging.h.in b/src/glog/logging.h.in index f897b2416..ceef5329c 100644 --- a/src/glog/logging.h.in +++ b/src/glog/logging.h.in @@ -88,6 +88,8 @@ #if HAVE_CXX11_ATOMIC #include +#elif defined(OS_WINDOWS) +#include #endif @ac_google_start_namespace@ @@ -937,12 +939,13 @@ PLOG_IF(FATAL, GOOGLE_PREDICT_BRANCH_NOT_TAKEN((invocation) == -1)) \ __FILE__, __LINE__, @ac_google_namespace@::GLOG_ ## severity, LOG_OCCURRENCES, \ &what_to_do).stream() -#else +#elif defined(OS_WINDOWS) #define SOME_KIND_OF_LOG_EVERY_N(severity, n, what_to_do) \ static int LOG_OCCURRENCES = 0, LOG_OCCURRENCES_MOD_N = 0; \ - ++LOG_OCCURRENCES; \ - if (++LOG_OCCURRENCES_MOD_N > n) LOG_OCCURRENCES_MOD_N -= n; \ + InterlockedIncrement(&LOG_OCCURRENCES); \ + if (InterlockedIncrement(&LOG_OCCURRENCES_MOD_N) > n) \ + InterlockedExchangeSubtract(&LOG_OCCURRENCES_MOD_N, n); \ if (LOG_OCCURRENCES_MOD_N == 1) \ @ac_google_namespace@::LogMessage( \ __FILE__, __LINE__, @ac_google_namespace@::GLOG_ ## severity, LOG_OCCURRENCES, \ @@ -950,7 +953,7 @@ PLOG_IF(FATAL, GOOGLE_PREDICT_BRANCH_NOT_TAKEN((invocation) == -1)) \ #define SOME_KIND_OF_LOG_IF_EVERY_N(severity, condition, n, what_to_do) \ static int LOG_OCCURRENCES = 0, LOG_OCCURRENCES_MOD_N = 0; \ - ++LOG_OCCURRENCES; \ + InterlockedIncrement(&LOG_OCCURRENCES); \ if (condition && \ ((LOG_OCCURRENCES_MOD_N=(LOG_OCCURRENCES_MOD_N + 1) % n) == (1 % n))) \ @ac_google_namespace@::LogMessage( \ @@ -959,8 +962,9 @@ PLOG_IF(FATAL, GOOGLE_PREDICT_BRANCH_NOT_TAKEN((invocation) == -1)) \ #define SOME_KIND_OF_PLOG_EVERY_N(severity, n, what_to_do) \ static int LOG_OCCURRENCES = 0, LOG_OCCURRENCES_MOD_N = 0; \ - ++LOG_OCCURRENCES; \ - if (++LOG_OCCURRENCES_MOD_N > n) LOG_OCCURRENCES_MOD_N -= n; \ + InterlockedIncrement(&LOG_OCCURRENCES); \ + if (InterlockedIncrement(&LOG_OCCURRENCES_MOD_N) > n) \ + InterlockedExchangeSubtract(&LOG_OCCURRENCES_MOD_N, n); \ if (LOG_OCCURRENCES_MOD_N == 1) \ @ac_google_namespace@::ErrnoLogMessage( \ __FILE__, __LINE__, @ac_google_namespace@::GLOG_ ## severity, LOG_OCCURRENCES, \ @@ -969,7 +973,47 @@ PLOG_IF(FATAL, GOOGLE_PREDICT_BRANCH_NOT_TAKEN((invocation) == -1)) \ #define SOME_KIND_OF_LOG_FIRST_N(severity, n, what_to_do) \ static int LOG_OCCURRENCES = 0; \ if (LOG_OCCURRENCES <= n) \ - ++LOG_OCCURRENCES; \ + InterlockedIncrement(&LOG_OCCURRENCES); \ + if (LOG_OCCURRENCES <= n) \ + @ac_google_namespace@::LogMessage( \ + __FILE__, __LINE__, @ac_google_namespace@::GLOG_ ## severity, LOG_OCCURRENCES, \ + &what_to_do).stream() + +#else + +#define SOME_KIND_OF_LOG_EVERY_N(severity, n, what_to_do) \ + static int LOG_OCCURRENCES = 0, LOG_OCCURRENCES_MOD_N = 0; \ + __sync_add_and_fetch(&LOG_OCCURRENCES, 1); \ + if (__sync_add_and_fetch(&LOG_OCCURRENCES_MOD_N, 1) > n) \ + __sync_sub_and_fetch(&LOG_OCCURRENCES_MOD_N, n); \ + if (LOG_OCCURRENCES_MOD_N == 1) \ + @ac_google_namespace@::LogMessage( \ + __FILE__, __LINE__, @ac_google_namespace@::GLOG_ ## severity, LOG_OCCURRENCES, \ + &what_to_do).stream() + +#define SOME_KIND_OF_LOG_IF_EVERY_N(severity, condition, n, what_to_do) \ + static int LOG_OCCURRENCES = 0, LOG_OCCURRENCES_MOD_N = 0; \ + __sync_add_and_fetch(&LOG_OCCURRENCES, 1); \ + if (condition && \ + ((LOG_OCCURRENCES_MOD_N=__sync_add_and_fetch(&LOG_OCCURRENCES_MOD_N, 1) % n) == (1 % n))) \ + @ac_google_namespace@::LogMessage( \ + __FILE__, __LINE__, @ac_google_namespace@::GLOG_ ## severity, LOG_OCCURRENCES, \ + &what_to_do).stream() + +#define SOME_KIND_OF_PLOG_EVERY_N(severity, n, what_to_do) \ + static int LOG_OCCURRENCES = 0, LOG_OCCURRENCES_MOD_N = 0; \ + __sync_add_and_fetch(&LOG_OCCURRENCES, 1); \ + if (__sync_add_and_fetch(&LOG_OCCURRENCES_MOD_N, n) > n) \ + __sync_sub_and_fetch(&LOG_OCCURRENCES_MOD_N, n); \ + if (LOG_OCCURRENCES_MOD_N == 1) \ + @ac_google_namespace@::ErrnoLogMessage( \ + __FILE__, __LINE__, @ac_google_namespace@::GLOG_ ## severity, LOG_OCCURRENCES, \ + &what_to_do).stream() + +#define SOME_KIND_OF_LOG_FIRST_N(severity, n, what_to_do) \ + static int LOG_OCCURRENCES = 0; \ + if (LOG_OCCURRENCES <= n) \ + __sync_add_and_fetch(&LOG_OCCURRENCES, 1); \ if (LOG_OCCURRENCES <= n) \ @ac_google_namespace@::LogMessage( \ __FILE__, __LINE__, @ac_google_namespace@::GLOG_ ## severity, LOG_OCCURRENCES, \ From bc3122036e6f01dbd96c00340669e52f01e5ab5d Mon Sep 17 00:00:00 2001 From: Marco Wang Date: Thu, 7 Nov 2019 12:58:26 +0800 Subject: [PATCH 3/4] src/glog/logging.h.in: fix wrong atomic increment --- src/glog/logging.h.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/glog/logging.h.in b/src/glog/logging.h.in index ceef5329c..92d5d212e 100644 --- a/src/glog/logging.h.in +++ b/src/glog/logging.h.in @@ -1003,7 +1003,7 @@ PLOG_IF(FATAL, GOOGLE_PREDICT_BRANCH_NOT_TAKEN((invocation) == -1)) \ #define SOME_KIND_OF_PLOG_EVERY_N(severity, n, what_to_do) \ static int LOG_OCCURRENCES = 0, LOG_OCCURRENCES_MOD_N = 0; \ __sync_add_and_fetch(&LOG_OCCURRENCES, 1); \ - if (__sync_add_and_fetch(&LOG_OCCURRENCES_MOD_N, n) > n) \ + if (__sync_add_and_fetch(&LOG_OCCURRENCES_MOD_N, 1) > n) \ __sync_sub_and_fetch(&LOG_OCCURRENCES_MOD_N, n); \ if (LOG_OCCURRENCES_MOD_N == 1) \ @ac_google_namespace@::ErrnoLogMessage( \ From 43430a4f480771b78dde08577a36e46828ba0603 Mon Sep 17 00:00:00 2001 From: Marco Wang Date: Thu, 7 Nov 2019 17:27:50 +0800 Subject: [PATCH 4/4] src/glog/logging.h.in: make modulo operations atomic --- src/glog/logging.h.in | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/glog/logging.h.in b/src/glog/logging.h.in index 92d5d212e..4471c44a8 100644 --- a/src/glog/logging.h.in +++ b/src/glog/logging.h.in @@ -955,7 +955,9 @@ PLOG_IF(FATAL, GOOGLE_PREDICT_BRANCH_NOT_TAKEN((invocation) == -1)) \ static int LOG_OCCURRENCES = 0, LOG_OCCURRENCES_MOD_N = 0; \ InterlockedIncrement(&LOG_OCCURRENCES); \ if (condition && \ - ((LOG_OCCURRENCES_MOD_N=(LOG_OCCURRENCES_MOD_N + 1) % n) == (1 % n))) \ + (InterlockedIncrement(&LOG_OCCURRENCES_MOD_N) || true) && \ + ((LOG_OCCURRENCES_MOD_N >= n && InterlockedExchangeAdd(&LOG_OCCURRENCES_MOD_N, n)) || true) && \ + LOG_OCCURRENCES_MOD_N == (1 % n)) \ @ac_google_namespace@::LogMessage( \ __FILE__, __LINE__, @ac_google_namespace@::GLOG_ ## severity, LOG_OCCURRENCES, \ &what_to_do).stream() @@ -995,7 +997,9 @@ PLOG_IF(FATAL, GOOGLE_PREDICT_BRANCH_NOT_TAKEN((invocation) == -1)) \ static int LOG_OCCURRENCES = 0, LOG_OCCURRENCES_MOD_N = 0; \ __sync_add_and_fetch(&LOG_OCCURRENCES, 1); \ if (condition && \ - ((LOG_OCCURRENCES_MOD_N=__sync_add_and_fetch(&LOG_OCCURRENCES_MOD_N, 1) % n) == (1 % n))) \ + (__sync_add_and_fetch(&LOG_OCCURRENCES_MOD_N, 1) || true) && \ + ((LOG_OCCURRENCES_MOD_N >= n && __sync_sub_and_fetch(&LOG_OCCURRENCES_MOD_N, n)) || true) && \ + LOG_OCCURRENCES_MOD_N == (1 % n)) \ @ac_google_namespace@::LogMessage( \ __FILE__, __LINE__, @ac_google_namespace@::GLOG_ ## severity, LOG_OCCURRENCES, \ &what_to_do).stream()