Locking a single bool variable when multithreading?

Question

Recently I have seen this code in a WebSite, and my question is the following:

        private bool mbTestFinished = false;

        private bool IsFinished()
        {
            lock( mLock )
            {
                return mbTestFinished;
            }
        }

        internal void SetFinished()
        {
            lock( mLock )
            {
                mbTestFinished = true;
            }
        }

In a multithread environment, is really necessary to lock the access to the mbTestFinished?

It is the most provable mechanism for ensuring it isn't a CPU-cached read (which would not work well between threads) - `volatile` would work too, but for reasons that are too complex (this isn't the *intent* of `volatile`, but rather: a side-effect) — Marc Gravell, Mar 30 '12 at 09:12
@MarcGravell I’ve always thought that _was_ the intent of `volatile`; any chance you might drop a good link that explains what is? — Roman Starkov, Mar 30 '12 at 09:21
@romkyns: This [answer](http://stackoverflow.com/a/4103879/158779) provides some insight. — Brian Gideon, Mar 30 '12 at 14:54
@BrianGideon Thanks! Having read that, it seems that either it _is_ the intended use, or `lock` also only accidentally achieves the desired effect, wouldn’t you agree? — Roman Starkov, Mar 30 '12 at 16:35
@romkyns: Well sort of. Except that `lock` actually *does* guarantee a fresh read because the memory barrier appears *before* the actual read. `volatile` puts the memory barrier *after* the read. I discuss this tangentially when explaining how `Thread.VolatileRead` is implemented at the bottom of my answer [here](http://stackoverflow.com/a/8417285/158779). — Brian Gideon, Mar 31 '12 at 03:21

score 13 · Answer 1 · answered Mar 30 '12 at 09:26

13

In my opinion, no, the lock is superfluous here, for two reasons:

Boolean variables cannot cause assignment tearing like long for example, hence locking is not necessary.
To solve the visibility problem volatile is enough. It's true that the lock introduces an implicit fence, but since the lock is not required for atomicity, volatile is enough.

answered Mar 30 '12 at 09:26

Tudor

61,523
12
102
142

1

This is true assuming the code shown is the *only* thing using the lock. If there are other places where `mLock` is taken, the conversion to `volatile` could break things. – Roman Starkov Mar 30 '12 at 09:28
@romkyns: Yes, I'm assuming it's a self-contained scenario. – Tudor Mar 30 '12 at 09:29

daryal · Accepted Answer · 2012-03-30T11:19:08.520

Yes, it is needed. .Net environment uses some optimizations, and sometimes if a memory location is accessed frequently, data is moved into CPU registers. So, in this case, if mbTestFinished is in a CPU register, then a thread reading it may get a wrong value. Thus using the volatile key ensures, all accesses to this variable is done at the memory location, not at the registers. On the otherhand, I have no idea of the frequency of this occurence. This may occur at a very low frequency.

score 7 · Answer 3 · answered Mar 30 '12 at 14:36

If mLock is ONLY for the variable mbTestFinished, then it's a bit of an overkill. Instead, you can use volatile or Interlocked, because both are User-Mode constructs for thread synchronization. lock (or Monitor) is a Hybrid Construct, in the sense that it is well optimized to avoid transiting from/to the Kernel-Mode whenever possible. The book "CLR via C#" has a in-depth discussion of these concepts.

Locking a single bool variable when multithreading?

3 Answers3