Call onRecycle after element is marked as deallocated in IndexedMemPool
authorVictor Zverovich <viz@fb.com>
Wed, 21 Jun 2017 18:38:32 +0000 (11:38 -0700)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Wed, 21 Jun 2017 18:42:36 +0000 (11:42 -0700)
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

folly/IndexedMemPool.h
folly/test/IndexedMemPoolTest.cpp

index 3a49b63..275d441 100644 (file)
@@ -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
index a5faa43..c15566b 100644 (file)
@@ -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));