ChunkedList: fix removal of unused chunks created through race in `pu…

…sh()` by decrementing its item count after each insertion of a new chunk

redGrapes/util/atomic_list.hpp

@@ -159,7 +159,7 @@ struct AtomicList
 
     /* initializes a new chunk
      */
-    void allocate_item()
+    auto allocate_item()
     {
         TRACE_EVENT("Allocator", "AtomicList::allocate_item()");
 
@@ -174,7 +174,7 @@ struct AtomicList
          */
         StaticAlloc<void> chunk_alloc( this->alloc, chunk_size );
         uintptr_t base = (uintptr_t)chunk_alloc.ptr;
-        append_item(
+        return append_item(
             std::allocate_shared< ItemPtr >(
                 chunk_alloc,
 
@@ -188,20 +188,6 @@ struct AtomicList
         );
     }
 
-    /* atomically appends a floating chunk to this list
-     */
-    void append_item( std::shared_ptr< ItemPtr > new_head )
-    {
-        TRACE_EVENT("Allocator", "AtomicList::append_item()");
-        bool append_successful = false;
-        while( ! append_successful )
-        {
-            std::shared_ptr< ItemPtr > old_head = std::atomic_load( &head );
-            std::atomic_store( &new_head->prev, old_head );
-            append_successful = std::atomic_compare_exchange_strong<ItemPtr>( &head, &old_head, new_head );
-        }
-    }
-
     template < bool is_const = false >
     struct BackwardIterator
     {
@@ -296,6 +282,27 @@ struct AtomicList
     {
         pos.erase();
     }
+
+    /* atomically appends a floating chunk to this list
+     * and returns the previous head to which the new_head
+     * is now linked.
+     */
+    auto append_item( std::shared_ptr< ItemPtr > new_head )
+    {
+        TRACE_EVENT("Allocator", "AtomicList::append_item()");
+        bool append_successful = false;
+        while( ! append_successful )
+        {
+            std::shared_ptr< ItemPtr > old_head = std::atomic_load( &head );
+            std::atomic_store( &new_head->prev, old_head );
+            append_successful = std::atomic_compare_exchange_strong<ItemPtr>( &head, &old_head, new_head );
+
+            if( append_successful )
+                return MutBackwardIterator{ old_head };
+        }
+    }
+
+
 };
 
 } // namespace memory

redGrapes/util/chunked_list.hpp

@@ -170,7 +170,7 @@ struct ChunkedList
          * compensated by not increasing the count when the last
          * element is inserted.
          */
-        std::atomic< chunk_offset_t > item_count{ 1 };
+        std::atomic< chunk_offset_t > item_count{ 0 };
 
         /* lowest index with free slot that can
          * be used to add a new element
@@ -435,6 +435,15 @@ struct ChunkedList
         spdlog::error("copy construct ChunkedList!!");
     }
 
+    /* decrement item_count and in case all items of this chunk are deleted,
+     * and this chunk is not `head`, delete the chunk too
+     */
+    void release_chunk( typename memory::AtomicList< Chunk, Allocator >::MutBackwardIterator chunk )
+    {
+        if( chunk->item_count.fetch_sub(1) == 0 )
+            chunks.erase( chunk );     
+    }
+
     MutBackwardIterator push( T const& item )
     {
         TRACE_EVENT("ChunkedList", "push");
@@ -448,16 +457,16 @@ struct ChunkedList
 
                 if( chunk_off < chunk_size )
                 {
-                    if( chunk_off+1 < chunk_size )
-                        chunk->item_count ++;
+                    chunk->item_count ++;
 
                     chunk->items()[ chunk_off ] = item;
                     chunk->last_idx ++;
                     return MutBackwardIterator( chunk_size, chunk, chunk_off );
                 }
             }
 
-            chunks.allocate_item();
+            auto prev_chunk = chunks.allocate_item();
+            release_chunk( prev_chunk );
         }
     }
 
@@ -484,14 +493,7 @@ struct ChunkedList
              */
             pos.item().remove();
 
-            /* in case all items of this chunk are deleted,
-             * delete the chunk too
-             */
-            if( pos.chunk->item_count.fetch_sub(1) == 1 )
-            {
-                // spdlog::info("last item!!");
-                chunks.erase( pos.chunk );
-            }
+            release_chunk( pos.chunk );
         }
         else
             throw std::runtime_error("remove invalid position");