The funny part is the SpinLock works as expected when its user are burst-compiled. However, As soon as I inserted a debug breakpoint in the code, the code fails, and multiple threads entered the spin-lock-protected code.
The question is:
What is the behaviour of Interlocked.CompareExchange() on a integer pointer inside mono?
Any chances of implementing any locks that have same behavior for both burst and mono in package com.unity.burst?
My spinlock works fine in both… Are you sure your spinlock is actually correct when not optimized and running instruction for instruction?
if (Interlocked.CompareExchange(ref *m_Access.m_Ptr,
(int) SpinLockState.Locked,
(int) SpinLockState.Unlocked) != m_Access.Value)
{
break;
}
So it assumes the lock is succesfully taken if “replace unlock with lock” returns that it found something other than “m_Access.Value”.
Shouldn’t it assume the lock is succesfully taken if “replace unlock with lock” returns that it found “unlock” ?
Would be nice to see the rest of the spinlock code/fields.
m_Access is pointer wrapper like NativeReference,
CompareExchange return the original value, then the m_Access.Value is accessed and compared. If the CompareExchange is successful, the original value and the current value of m_Access will be different.
You mentioned “not optimized and running instruction for instruction?” Do you mean C# optimization? How should I know the that is the actually being executed?
the rest of the spinlock code would look something like these
What I mean is that when you run the code with your debugger attached to unity, what runs is sort of unoptimized code that is closer to your actual written code for a better debugging experience. When you run in release mode without a debugger, the code is more optimized and may do unexpected things compared to what you actually write, as long as it matches the spec. Slower running code also tends to showcase threading bugs much more clearly than quick running code (since contention is higher when it runs slower).
You’re reading the value twice - once from the atomic CompareExchange and once from the regular read, and then comparing those two. The whole of those 3 (atomic read, read and compare) isn’t safe.
Say these 3 ops go in this order:
CompareExchange, return found value
Read value
Compare found value with read value, assume locked if different
Let’s say the lock is taken by thread A as a start.
Thread B tries to take the lock with a CompareExchange, and fails, returns “locked”. For whatever reason thread B is suspended/sleeps/stalls for a moment.
Thread A unlocks the lock.
Thread B reads the value, reads “unlocked”
Thread B compares the CompareExchange result “locked” to the read value “unlocked”, and assumes it took the lock.
Now the lock is marked non-taken yet thread B is running the mutually exclusive code.
What you want to do instead of reading the value separately from the CompareExchange is to use the value that CompareExchange returned. If it returns “unlocked”, it has written “locked”, and you can consider the lock taken and stop the loop. If it returns “locked”, it didn’t do anything, and you retry.