Add previous / current thread to collision warner

The failure message is now more useful and looks like the following: F0206 20:31:24.554322 30972 thread_collision_warner.cc:23] Thread Collision! Previous thread id: 30962, current thread id: 30972 I found this necessary when debugging a particular issue since by the time the SIGABRT triggered in GDB the previous thread had already exited the critical section (perhaps while printing the fatal log message). Change-Id: I038bad901b5235725084bd8671ef4b02ec22c0a7 Reviewed-on: http://gerrit.cloudera.org:8080/9237 Reviewed-by: Alexey Serbin <[email protected]> Tested-by: Kudu Jenkins
apache · Feb 7, 2018 · be16a81 · be16a81
1 parent cfecb1c
commit be16a81
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 15 deletions.
diff --git a/src/kudu/gutil/threading/thread_collision_warner.cc b/src/kudu/gutil/threading/thread_collision_warner.cc
@@ -19,8 +19,9 @@
 
 namespace base {
 
-void DCheckAsserter::warn() {
-  LOG(FATAL) << "Thread Collision";
+void DCheckAsserter::warn(int64_t previous_thread_id, int64_t current_thread_id) {
+  LOG(FATAL) << "Thread Collision! Previous thread id: " << previous_thread_id
+             << ", current thread id: " << current_thread_id;
 }
 
 #if 0
@@ -57,13 +58,13 @@ void ThreadCollisionWarner::EnterSelf() {
   // write on valid_thread_id_ the current thread ID.
   subtle::Atomic64 current_thread_id = CurrentThread();
 
-  int64_t previous_value = subtle::NoBarrier_CompareAndSwap(&valid_thread_id_,
-                                                            0,
-                                                            current_thread_id);
-  if (previous_value != 0 && previous_value != current_thread_id) {
+  int64_t previous_thread_id = subtle::NoBarrier_CompareAndSwap(&valid_thread_id_,
+                                                                0,
+                                                                current_thread_id);
+  if (previous_thread_id != 0 && previous_thread_id != current_thread_id) {
     // gotcha! a thread is trying to use the same class and that is
     // not current thread.
-    asserter_->warn();
+    asserter_->warn(previous_thread_id, current_thread_id);
   }
 
   subtle::NoBarrier_AtomicIncrement(&counter_, 1);
@@ -72,11 +73,12 @@ void ThreadCollisionWarner::EnterSelf() {
 void ThreadCollisionWarner::Enter() {
   subtle::Atomic64 current_thread_id = CurrentThread();
 
-  if (subtle::NoBarrier_CompareAndSwap(&valid_thread_id_,
-                                       0,
-                                       current_thread_id) != 0) {
+  int64_t previous_thread_id = subtle::NoBarrier_CompareAndSwap(&valid_thread_id_,
+                                                                0,
+                                                                current_thread_id);
+  if (previous_thread_id != 0) {
     // gotcha! another thread is trying to use the same class.
-    asserter_->warn();
+    asserter_->warn(previous_thread_id, current_thread_id);
   }
 
   subtle::NoBarrier_AtomicIncrement(&counter_, 1);

diff --git a/src/kudu/gutil/threading/thread_collision_warner.h b/src/kudu/gutil/threading/thread_collision_warner.h
@@ -5,10 +5,11 @@
 #ifndef BASE_THREADING_THREAD_COLLISION_WARNER_H_
 #define BASE_THREADING_THREAD_COLLISION_WARNER_H_
 
+#include <cstdint>
+
 #include "kudu/gutil/atomicops.h"
 #include "kudu/gutil/macros.h"
-#include "kudu/gutil/port.h"
-
+
 #ifndef BASE_EXPORT
 #define BASE_EXPORT
 #endif
@@ -135,12 +136,12 @@ namespace base {
 // in case of collision (check thread_collision_warner_unittests.cc)
 struct BASE_EXPORT AsserterBase {
   virtual ~AsserterBase() {}
-  virtual void warn() = 0;
+  virtual void warn(int64_t previous_thread_id, int64_t current_thread_id) = 0;
 };
 
 struct BASE_EXPORT DCheckAsserter : public AsserterBase {
   virtual ~DCheckAsserter() {}
-  virtual void warn() OVERRIDE;
+  void warn(int64_t previous_thread_id, int64_t current_thread_id) override;
 };
 
 class BASE_EXPORT ThreadCollisionWarner {