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 3a49b634ff86af471a82dde082c8348872f00bea..275d441cc0e646746bd1f142f257853050d17d7e 100644 (file)
--- 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 a5faa43ada334c3c3fc215d4958c0078f7307675..c15566b29def444d48a43c1c4fe8c4da41762f12 100644 (file)
--- 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));