Saw the ASSERT_HOLDING_SPIN_LOCK fail in the region allocator. (#54896)

I think this is due to a race between a thread leaving the lock and a thread entering the lock. The thread entering the lock sets the holding_thread to itself, and the thread leaving the lock sets it to -1. The thread leaving the lock is already outside of the lock, and so its write may occur after the write by the thread entering the lock.

The end result is that the lock appears taken, but with a holding_thread of -1, which causes the assert.

Swapping the order of the two writes will fix the issue, because now the write to holding_thread is under the lock, so two threads cannot do conflicting writes.

diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp
index ea9b6fc..1d58d35 100644 (file)
--- a/src/coreclr/gc/gc.cpp
+++ b/src/coreclr/gc/gc.cpp
@@ -3645,10 +3645,10 @@ void region_allocator::enter_spin_lock()
 
 void region_allocator::leave_spin_lock()
 {
-    region_allocator_lock.lock = -1;
 #ifdef _DEBUG
     region_allocator_lock.holding_thread = (Thread*)-1;
 #endif //_DEBUG
+    region_allocator_lock.lock = -1;
 }
 
 uint8_t* region_allocator::allocate (uint32_t num_units, allocate_direction direction)