From a32ff6eb69fa72057057495d59363d7827c4f55b Mon Sep 17 00:00:00 2001
From: Christian Kern <christian.kern@siemens.com>
Date: Tue, 7 Oct 2014 11:09:01 +0200
Subject: [PATCH] Fixed bug due to Hazard Pointer. HelpScan was implemented wrong -> Scan could be executed be different threads the same time -> FixedSize lists got messed up -> program tried to return wrong pointers to the object pool -> segfault or hang

Moreover, added assertion that checks that only one thread concurrently enters scan, for avoiding these kind of bugs in the future.
---
 containers_cpp/include/embb/containers/internal/hazard_pointer-inl.h | 36 +++++++++++++++++++++++++++---------
 containers_cpp/include/embb/containers/internal/hazard_pointer.h     | 12 +++++++++++-
 2 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/containers_cpp/include/embb/containers/internal/hazard_pointer-inl.h b/containers_cpp/include/embb/containers/internal/hazard_pointer-inl.h
index 2877554..a8b4e50 100644
--- a/containers_cpp/include/embb/containers/internal/hazard_pointer-inl.h
+++ b/containers_cpp/include/embb/containers/internal/hazard_pointer-inl.h
@@ -144,11 +144,20 @@ template< typename GuardType >
 HazardPointerThreadEntry<GuardType>::
 HazardPointerThreadEntry(GuardType undefined_guard, int guards_per_thread,
   size_t max_size_retired_list) :
+#ifdef EMBB_DEBUG
+  who_is_scanning(-1),
+#endif
   undefined_guard(undefined_guard),
   guards_per_thread(guards_per_thread),
   max_size_retired_list(max_size_retired_list),
   retired_list(max_size_retired_list),
   retired_list_temp(max_size_retired_list),
+  // initially, each potential thread is active... if that is not the case
+  // another thread could call "HelpScan", and block this thread in making 
+  // progress.
+  // Still, threads can be leave the hazard pointer processing (deactivation),
+  // but this can only be done once, i.e., this is not revertable...
+  is_active(1),
   hazard_pointer_list_temp(embb::base::Thread::GetThreadsMaxCount() *
     guards_per_thread) {
   // Initialize guarded pointer list
@@ -190,7 +199,7 @@ GuardPointer(int guardNumber, GuardType pointerToGuard) {
 template< typename GuardType >
 void HazardPointerThreadEntry<GuardType>::SetActive(bool active) {
   if (active == true)
-  is_active = 1;
+    is_active = 1;
   else
     is_active = 0;
 }
@@ -231,16 +240,9 @@ HazardPointer< GuardType >::GetHazardPointerElementForCurrentThread() {
   // stop operating, and the others are responsible for his retired
   // list.
 
-  // int expected = false;
   HazardPointerThreadEntry_t* current_thread_entry =
     &hazard_pointer_thread_entry_array[GetCurrentThreadIndex()];
 
-  // If not active, activate it
-  if (!current_thread_entry->IsActive()) {
-    current_thread_entry->SetActive(true);
-    active_hazard_pointer++;
-  }
-
   return hazard_pointer_thread_entry_array[GetCurrentThreadIndex()];
 }
 
@@ -270,6 +272,16 @@ void HazardPointer< GuardType >::HelpScan() {
 template< typename GuardType >
 void HazardPointer< GuardType >::
 Scan(HazardPointerThreadEntry_t* currentHazardPointerEntry) {
+#ifdef EMBB_DEBUG
+  // scan should only be executed by one thread at a time, otherwise we have
+  // a bug... this assertions checks that
+  int expected = -1;
+  if (!currentHazardPointerEntry->GetScanningThread().CompareAndSwap(
+    expected, GetCurrentThreadIndex()))
+  {
+    assert(false);
+  }
+#endif
   // In this function, we compute the intersection between local retired
   // pointers and all hazard pointers. This intersection cannot be deleted and
   // forms the new local retired pointers list.
@@ -320,6 +332,10 @@ Scan(HazardPointerThreadEntry_t* currentHazardPointerEntry) {
   }
   currentHazardPointerEntry->SetRetired(
     currentHazardPointerEntry->GetRetiredTemp());
+
+#ifdef EMBB_DEBUG
+  currentHazardPointerEntry->GetScanningThread().Store(-1);
+#endif
 }
 
 template< typename GuardType >
@@ -335,7 +351,8 @@ HazardPointer< GuardType >::HazardPointer(
   GuardType undefined_guard, int guards_per_thread) :
   undefined_guard(undefined_guard),
   guards_per_thread(guards_per_thread),
-  active_hazard_pointer(0),
+  //initially, all potential hazard pointers are active...
+  active_hazard_pointer(embb::base::Thread::GetThreadsMaxCount()),
   free_guard_callback(free_guard_callback) {
   hazard_pointers = embb::base::Thread::GetThreadsMaxCount();
 
@@ -388,6 +405,7 @@ void HazardPointer< GuardType >::EnqueuePointerForDeletion(
   if (IsThresholdExceeded()) {
     HazardPointerThreadEntry_t* currentHazardPointerEntry =
       &GetHazardPointerElementForCurrentThread();
+
     Scan(currentHazardPointerEntry);
 
     // Help deactivated threads to clean their retired nodes.
diff --git a/containers_cpp/include/embb/containers/internal/hazard_pointer.h b/containers_cpp/include/embb/containers/internal/hazard_pointer.h
index 1d3ded3..9a63823 100644
--- a/containers_cpp/include/embb/containers/internal/hazard_pointer.h
+++ b/containers_cpp/include/embb/containers/internal/hazard_pointer.h
@@ -169,7 +169,17 @@ class FixedSizeList {
  */
 template< typename GuardType >
 class HazardPointerThreadEntry {
- private:
+
+#ifdef EMBB_DEBUG
+  public:
+    embb::base::Atomic<int>& GetScanningThread()
+    {
+      return who_is_scanning;
+    }
+  private:
+    embb::base::Atomic<int> who_is_scanning;
+#endif
+  private:
    /**
     * Value of the undefined guard (means that no guard is set).
     */
--
libgit2 0.26.0