From 3efbe80ee12f522147f89da3ffde3d03764c8089 Mon Sep 17 00:00:00 2001 From: Christian Kern Date: Fri, 10 Oct 2014 13:39:43 +0200 Subject: [PATCH] For the michael scott queue, follow the exact implementation of Paper: Michael, Maged M. "Hazard pointers: Safe memory reclamation for lock-free objects." Parallel and Distributed Systems, IEEE Transactions on 15.6 (2004): 491-504. Might fix bug, where the MPMC queue hang (very very spuriously), because tail->GetNext() == tail... --- containers_cpp/include/embb/containers/internal/lock_free_mpmc_queue-inl.h | 89 ++++++++++++++++++++++++++++++++++------------------------------------------------------- containers_cpp/include/embb/containers/lock_free_mpmc_queue.h | 18 ++++++++---------- 2 files changed, 42 insertions(+), 65 deletions(-) diff --git a/containers_cpp/include/embb/containers/internal/lock_free_mpmc_queue-inl.h b/containers_cpp/include/embb/containers/internal/lock_free_mpmc_queue-inl.h index 9ee96da..e4fb9ae 100644 --- a/containers_cpp/include/embb/containers/internal/lock_free_mpmc_queue-inl.h +++ b/containers_cpp/include/embb/containers/internal/lock_free_mpmc_queue-inl.h @@ -116,28 +116,28 @@ bool LockFreeMPMCQueue::TryEnqueue(T const& element) { internal::LockFreeMPMCQueueNode* my_tail; for (;;) { my_tail = tail; - internal::LockFreeMPMCQueueNode* my_tail_next = my_tail->GetNext(); hazardPointer.GuardPointer(0, my_tail); // Check if pointer is still valid after guarding. if (my_tail != tail) { - hazardPointer.GuardPointer(0, NULL); continue; // Hazard pointer outdated, retry } + internal::LockFreeMPMCQueueNode* my_tail_next = my_tail->GetNext(); + if (my_tail == tail) { // If the next pointer of the tail node is null, the tail pointer // points to the last object. We try to set the next pointer of the // tail node to our new node. if (my_tail_next == NULL) { + internal::LockFreeMPMCQueueNode* expected = NULL; // This fails if the next pointer of the "cached" tail is not null // anymore, i.e., another thread added a node before we could complete. - if (my_tail->GetNext().CompareAndSwap(my_tail_next, node)) - - // We successfully added our node. Still missing: increase tail pointer. - break; - // The tail pointer points not to the last object, first increase + if (my_tail->GetNext().CompareAndSwap(expected, node)) + break; // We successfully added our node. + //Still missing: increase tail pointer. + // The tail pointer points not to the last object, first increase } else { // Try to increase the tail pointer. tail.CompareAndSwap(my_tail, my_tail_next); @@ -147,66 +147,45 @@ bool LockFreeMPMCQueue::TryEnqueue(T const& element) { // We added our node. Try to update tail pointer. Need not succeed, if we // fail, another thread will help us. tail.CompareAndSwap(my_tail, node); - // Release guard - hazardPointer.GuardPointer(0, NULL); return true; } template< typename T, typename ValuePool > bool LockFreeMPMCQueue::TryDequeue(T & element) { - T value; + internal::LockFreeMPMCQueueNode* my_head; + internal::LockFreeMPMCQueueNode* my_tail; + internal::LockFreeMPMCQueueNode* my_next; + internal::LockFreeMPMCQueueNode* expected; + T data; for (;;) { - internal::LockFreeMPMCQueueNode* my_head = head; - internal::LockFreeMPMCQueueNode* my_tail = tail; - internal::LockFreeMPMCQueueNode* my_head_next = my_head->GetNext(); - - // Head did not change - if (my_head == head) { - // Guard head - hazardPointer.GuardPointer(0, my_head); - - // Check if pointer is still valid after guarding. This check is - // essential, tests really crash if missing - if (my_head != head) { - hazardPointer.GuardPointer(0, NULL); - continue; // Hazard pointer outdated, retry - } - - // Check if pointer is still valid after guarding. This check is - // essential, tests really crash if missing - hazardPointer.GuardPointer(1, my_head_next); - if (my_head_next != my_head->GetNext()) { - hazardPointer.GuardPointer(1, NULL); - continue; // Hazard pointer outdated, retry - } + my_head = head; + hazardPointer.GuardPointer(0, my_head); + if (my_head != head) continue; - if (my_tail == my_head) { - if (my_head_next == NULL) { - // Queue is empty. Release guards and return false. + my_tail = tail; + my_next = my_head->GetNext(); + hazardPointer.GuardPointer(1, my_next); + if (head != my_head) continue; - hazardPointer.GuardPointer(0, NULL); - hazardPointer.GuardPointer(1, NULL); + if (my_next == NULL) + return false; - // Queue is empty; - return false; - } - // Tail is not pointing to last element, help to increase - tail.CompareAndSwap(my_head, my_head_next); - } else { - value = my_head_next->GetElement(); - if (head.CompareAndSwap(my_head, my_head_next)) { - // It's our element. Release guard and enqueue my_head for - // deletion and leave. - hazardPointer.GuardPointer(0, NULL); - hazardPointer.GuardPointer(1, NULL); - hazardPointer.EnqueuePointerForDeletion(my_head); - break; - } - } + if (my_head == my_tail) { + expected = my_tail; + tail.CompareAndSwap(expected, my_next); + continue; } + + data = my_next->GetElement(); + + expected = my_head; + if (head.CompareAndSwap(expected, my_next)) + break; } - element = value; + + hazardPointer.EnqueuePointerForDeletion(my_head); + element = data; return true; } } // namespace containers diff --git a/containers_cpp/include/embb/containers/lock_free_mpmc_queue.h b/containers_cpp/include/embb/containers/lock_free_mpmc_queue.h index 16cce38..ae1a232 100644 --- a/containers_cpp/include/embb/containers/lock_free_mpmc_queue.h +++ b/containers_cpp/include/embb/containers/lock_free_mpmc_queue.h @@ -74,8 +74,7 @@ class LockFreeMPMCQueueNode { */ LockFreeMPMCQueueNode( T const& element - /**< [IN] The element of this queue node */ - ); + /**< [IN] The element of this queue node */); /** * Returns the next pointer @@ -121,14 +120,14 @@ class LockFreeMPMCQueue { * Callback to the method that is called by hazard pointers if a pointer is * not hazardous anymore, i.e., can safely be reused. */ - embb::base::Function*> + embb::base::Function < void, internal::LockFreeMPMCQueueNode* > delete_pointer_callback; /** * The hazard pointer object, used for memory management. */ - embb::containers::internal::HazardPointer*> - hazardPointer; + embb::containers::internal::HazardPointer + < internal::LockFreeMPMCQueueNode* > hazardPointer; /** * The object pool, used for lock-free memory allocation. @@ -200,8 +199,7 @@ class LockFreeMPMCQueue { */ bool TryEnqueue( T const& element - /**< [IN] Const reference to the element that shall be enqueued */ - ); + /**< [IN] Const reference to the element that shall be enqueued */); /** * Tries to dequeue an element from the queue. @@ -215,9 +213,9 @@ class LockFreeMPMCQueue { */ bool TryDequeue( T & element - /**< [IN,OUT] Reference to the dequeued element. Unchanged, if the operation - was not successful. */ - ); + /**< [IN, OUT] Reference to the dequeued element. + Unchanged, if the operation + was not successful. */); }; } // namespace containers } // namespace embb -- libgit2 0.26.0