From: Victor Zverovich <viz@fb.com>
Date: Wed, 21 Jun 2017 18:38:32 +0000 (-0700)
Subject: Call onRecycle after element is marked as deallocated in IndexedMemPool
X-Git-Tag: v2017.06.26.00~24
X-Git-Url: http://plrg.eecs.uci.edu/git/?p=folly.git;a=commitdiff_plain;h=15884e47719c8b4e5f3c5ea51f50aebb46887611

Call onRecycle after element is marked as deallocated in IndexedMemPool

Summary: Make `IndexedMemPool` call `Traits::onRecycle` on the element just before it is marked as deallocated. This mirrors the allocation behavior implemented in D5177462 and simplifies preventing access to recycled elements (the client just needs to check `isAllocated` before accessing the element).

Reviewed By: nbronson

Differential Revision: D5275283

fbshipit-source-id: 58365b5b7b32b07fa56529c476078f241fc20811
---

diff --git a/folly/IndexedMemPool.h b/folly/IndexedMemPool.h
index 3a49b634..275d441c 100644
--- a/folly/IndexedMemPool.h
+++ b/folly/IndexedMemPool.h
@@ -259,7 +259,6 @@ struct IndexedMemPool : boost::noncopyable {
   /// Gives up ownership previously granted by alloc()
   void recycleIndex(uint32_t idx) {
     assert(isAllocated(idx));
-    Traits::onRecycle(&slot(idx).elem);
     localPush(localHead(), idx);
   }
 
@@ -422,7 +421,8 @@ struct IndexedMemPool : boost::noncopyable {
     Slot& s = slot(idx);
     TaggedPtr h = head.load(std::memory_order_acquire);
     while (true) {
-      s.localNext.store(h.idx, std::memory_order_relaxed);
+      s.localNext.store(h.idx, std::memory_order_release);
+      Traits::onRecycle(&slot(idx).elem);
 
       if (h.size() == LocalListLimit) {
         // push will overflow local list, steal it instead
diff --git a/folly/test/IndexedMemPoolTest.cpp b/folly/test/IndexedMemPoolTest.cpp
index a5faa43a..c15566b2 100644
--- a/folly/test/IndexedMemPoolTest.cpp
+++ b/folly/test/IndexedMemPoolTest.cpp
@@ -357,7 +357,7 @@ void testTraits(TraitsTestPool& pool) {
 
   elem = nullptr;
   EXPECT_CALL(traits, onRecycle(_)).WillOnce(Invoke([&](std::string* s) {
-    EXPECT_TRUE(pool.isAllocated(pool.locateElem(s)));
+    EXPECT_FALSE(pool.isAllocated(pool.locateElem(s)));
     elem = s;
   }));
   pool.recycleIndex(pool.locateElem(ptr));