Question: suppose I have xml inside an html document like

```
<html>
  <body>
     <xml>
        <foo>
          <bar />
          <baz />
        </foo>
     </xml>
   </body>
 </html>

```

What is the best way to apply xslt to the xml node?

Answer: 
It looks to me like this is an attempt at optimisation gone slightly wrong. I *suspect* it's trying for "TATAS" - "Test-and-test-and-set", where it doesn't even try to do the TestAndSet if it can see the lock is already taken.

In a [post about spin locks for .NET](http://www.bluebytesoftware.com/blog/CommentView,guid,72aaa68e-4cbd-41db-a7ce-ddc64411eafa.aspx), Joe Duffy writes this TATAS code as:

```
class SpinLock {
    private volatile int m_taken;

    public void Enter() {
        while (true) {
            if (m_taken == 0 &&
                    Interlocked.Exchange(ref m_taken, 1) == 0)
                break;
        }
    }

    public void Exit() {
        m_taken = 0;
    }
}

```

(Note that Joe is using 1 to mean locked and 0 to mean unlocked, unlike the code project sample - either is fine, just don't get confused between the two!)

Note that here the call to Interlocked.Exchange *is conditional on `m_taken` being 0*. This reduces contention - the relatively expensive (I guess) test-and-set operation is avoided there it's unnecessary. I *suspect* that's what the author was aiming at, but didn't quite get it right.

This is also mentioned in the [Wikipedia article about spinlocks](http://en.wikipedia.org/wiki/Spin_locking) under "significant optimisations":

> 
> To reduce inter-CPU bus traffic, when
>  the lock is not acquired, the code
>  should loop reading without trying to
>  write anything, until it reads a
>  changed value. Because of MESI caching
>  protocols, this causes the cache line
>  for the lock to become "Shared"; then
>  there is remarkably no bus traffic
>  while a CPU is waiting for the lock.
>  This optimization is effective on all
>  CPU architectures that have a cache
>  per CPU, because MESI is so
>  ubiquitous.
> 
> 
> 

That "loop reading" is exactly what the while loop does - until it sees `m_taken` change, it only reads. When it sees the change (i.e. when the lock is released) it has another go at locking.

Of course it's very possible that I'm missing something important - issues like this are very subtle.
Why both conditions? Because a second thread will also get the lock in that case. *(Edit: but that case can't happen if all threads follow the spinlock protocol.)*

If the lock is available (signaled) `m_s` has the value 1. When taken by some thread, it has the value 0. No other values are allowed.

Consider one thread that wants the lock, whether or not it is signaled at the moment that thread called `Enter()` is unimportant. It is allowed to take the lock if `m_s` is 1, and it takes by changing it to 0. The first iteration where that happens causes the loop to exit and the thread to have the lock.

Now consider two threads that want the same lock. Both are calling `TestAndSet()`, waiting to see the value 1 become 0. Because the function `TestAndSet()` is atomic, only one of the waiting threads ever gets to see the value 1. All the other threads only ever see `m_s` as 0, and must continue to wait.

The condition where `m_s` is 1 after setting it to 0 in this thread implies that some other thread signaled between the atomic operation and the condition. Since only one thread at a time is supposed to have the lock, it seems like it shouldn't happen that can't happen. 

I'm guessing that this is an attempt to satisfy the invariant promise of the spinlock. *(Edit: I'm no longer so sure, more below...)* If it is held, the value of `m_s` must be zero. If not, it is one. If setting it to zero didn't "stick", then something weird is happening, and it is best not to assume it is now held by this thread when the invariant is not true.

**Edit:** Jon Skeet points out that this case might be a flaw in the original implementation. I suspect he's right. 

The race condition that is being guarded is against a thread that **doesn't have the right to signal the spinlock**, signaling the spinlock anyway. If you can't trust callers to follow the rules, spinlocks probably aren't the synchronization method of choice, after all.

**Edit 2:** The proposed revision looks much better. It would clearly avoid the multi-core cache coherency interaction that the original had due to always writing the sentinel `m_s`. 

After reading about the [TATAS protocol](http://en.wikipedia.org/wiki/Test_and_Test-and-set) (you can learn something new every day, if you pay attention...) and the multicore cache coherency issue it is addressing, it is clear to me that the original code was trying to do something similar, but without understanding the subtlety behind it. It would indeed have been safe (assuming all callers follow the rules) to drop the redundant check on `m_s` as it was written. But the code would have been writing to `m_s` on every loop iteration, and that would have played havoc in a real multicore chip with caching per core.

The new spinlock is still vulnerable to a second thread releasing it without holding it. There is no way to repair that. My earlier claim about trusting the callers to follow protocol still applies.