[utils] Fix retiring of live block for the lock free allocator

Freeing a block descriptor (along with the block) can happen either when freeing a pointer in the block (we do this in order to prevent situations where we need to wait for the next alloc in order to free a block), either when we acquire an empty block while allocating. Either way, in order to free a block descriptor, we need to acquire it first.

When freeing a slot in a block, we are checking if the block is now empty in which case we acquire the block descriptor in order to free it (along with the block). While doing so, we were failing to account for the case where, by the time we make the block descriptor empty and the time we acquire it, another thread would try to allocate from the same descriptor, see that it's empty, free it, allocate a new descriptor (which would happen to have the same address as the retired one) to fulfill the allocation and then set it as the active descriptor for the allocator. When the first thread finally acquires the descriptor, we need to double check that it is still empty since it might be logically a different descriptor than the one we actually freed from. In the unlikely case it's not empty, we need to put it back in the allocator's data structures.

diff --git a/mono/utils/lock-free-alloc.c b/mono/utils/lock-free-alloc.c
index 3be8342a49873dbc9e2796d44a76a81d5a9302e3..d103ef6aa333eeb96a920d8e62ef0af79db5ecb2 100644 (file)
--- a/mono/utils/lock-free-alloc.c
+++ b/mono/utils/lock-free-alloc.c
@@ -477,8 +477,18 @@ mono_lock_free_free (gpointer ptr, size_t block_size)
                g_assert (old_anchor.data.state != STATE_EMPTY);
 
                if (InterlockedCompareExchangePointer ((gpointer * volatile)&heap->active, NULL, desc) == desc) {
-                       /* We own it, so we free it. */
-                       desc_retire (desc);
+                       /*
+                        * We own desc, check if it's still empty, in which case we retire it.
+                        * If it's partial we need to put it back either on the active slot or
+                        * on the partial list.
+                        */
+                       if (desc->anchor.data.state == STATE_EMPTY) {
+                               desc_retire (desc);
+                       } else if (desc->anchor.data.state == STATE_PARTIAL) {
+                               if (InterlockedCompareExchangePointer ((gpointer * volatile)&heap->active, desc, NULL) != NULL)
+                                       heap_put_partial (desc);
+
+                       }
                } else {
                        /*
                         * Somebody else must free it, so we do some