Commit 13681962 authored by Maged Michael's avatar Maged Michael Committed by Facebook Github Bot

hazptr: Clear hazard pointers in ~hazptr_array() and ~hazptr_local()

Summary:
Clear hazard pointers in ~hazptr_array() and ~hazptr_local().
Added tests to detect uncleared hazard pointers when using hazptr_array and hazptr_local.

Differential Revision: D6995057

fbshipit-source-id: 5b8fa1d197c3959ed670d0a019959bd57f5df3d0
parent 49f93b0d
...@@ -721,6 +721,7 @@ FOLLY_ALWAYS_INLINE hazptr_array<M>::~hazptr_array() { ...@@ -721,6 +721,7 @@ FOLLY_ALWAYS_INLINE hazptr_array<M>::~hazptr_array() {
auto count = tc.count(); auto count = tc.count();
if ((M <= HAZPTR_TC_SIZE) && (count + M <= HAZPTR_TC_SIZE)) { if ((M <= HAZPTR_TC_SIZE) && (count + M <= HAZPTR_TC_SIZE)) {
for (size_t i = 0; i < M; ++i) { for (size_t i = 0; i < M; ++i) {
h[i].reset();
tc[count + i].hprec_ = h[i].hazptr_; tc[count + i].hprec_ = h[i].hazptr_;
HAZPTR_DEBUG_PRINT(i << " " << &h[i]); HAZPTR_DEBUG_PRINT(i << " " << &h[i]);
new (&h[i]) hazptr_holder(nullptr); new (&h[i]) hazptr_holder(nullptr);
...@@ -793,7 +794,7 @@ FOLLY_ALWAYS_INLINE hazptr_local<M>::hazptr_local() { ...@@ -793,7 +794,7 @@ FOLLY_ALWAYS_INLINE hazptr_local<M>::hazptr_local() {
} }
} }
// Slow path // Slow path
need_destruct_ = true; slow_path_ = true;
for (size_t i = 0; i < M; ++i) { for (size_t i = 0; i < M; ++i) {
new (&h[i]) hazptr_holder; new (&h[i]) hazptr_holder;
HAZPTR_DEBUG_PRINT( HAZPTR_DEBUG_PRINT(
...@@ -803,7 +804,7 @@ FOLLY_ALWAYS_INLINE hazptr_local<M>::hazptr_local() { ...@@ -803,7 +804,7 @@ FOLLY_ALWAYS_INLINE hazptr_local<M>::hazptr_local() {
template <size_t M> template <size_t M>
FOLLY_ALWAYS_INLINE hazptr_local<M>::~hazptr_local() { FOLLY_ALWAYS_INLINE hazptr_local<M>::~hazptr_local() {
if (LIKELY(!need_destruct_)) { if (LIKELY(!slow_path_)) {
if (kIsDebug) { if (kIsDebug) {
auto ptc = hazptr_tc_tls(); auto ptc = hazptr_tc_tls();
DCHECK(ptc != nullptr); DCHECK(ptc != nullptr);
...@@ -811,6 +812,10 @@ FOLLY_ALWAYS_INLINE hazptr_local<M>::~hazptr_local() { ...@@ -811,6 +812,10 @@ FOLLY_ALWAYS_INLINE hazptr_local<M>::~hazptr_local() {
DCHECK(tc.local_); DCHECK(tc.local_);
tc.local_ = false; tc.local_ = false;
} }
auto h = reinterpret_cast<hazptr_holder*>(&raw_);
for (size_t i = 0; i < M; ++i) {
h[i].reset();
}
return; return;
} }
// Slow path // Slow path
......
...@@ -328,7 +328,7 @@ class hazptr_local { ...@@ -328,7 +328,7 @@ class hazptr_local {
private: private:
aligned_hazptr_holder raw_[M]; aligned_hazptr_holder raw_[M];
bool need_destruct_{false}; bool slow_path_{false};
}; };
} // namespace hazptr } // namespace hazptr
......
...@@ -639,4 +639,38 @@ TEST_F(HazptrTest, FreeFunctionCleanup) { ...@@ -639,4 +639,38 @@ TEST_F(HazptrTest, FreeFunctionCleanup) {
for (auto& t : threads) { for (auto& t : threads) {
t.join(); t.join();
} }
{ // Cleanup after using array
constructed.store(0);
destroyed.store(0);
{ hazptr_array<2> h; }
{
hazptr_array<2> h;
auto p0 = new Foo(0, nullptr);
auto p1 = new Foo(0, nullptr);
h[0].reset(p0);
h[1].reset(p1);
p0->retire();
p1->retire();
}
CHECK_EQ(constructed.load(), 2);
hazptr_cleanup();
CHECK_EQ(destroyed.load(), 2);
}
{ // Cleanup after using local
constructed.store(0);
destroyed.store(0);
{ hazptr_local<2> h; }
{
hazptr_local<2> h;
auto p0 = new Foo(0, nullptr);
auto p1 = new Foo(0, nullptr);
h[0].reset(p0);
h[1].reset(p1);
p0->retire();
p1->retire();
}
CHECK_EQ(constructed.load(), 2);
hazptr_cleanup();
CHECK_EQ(destroyed.load(), 2);
}
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment