C# ThreadStatic + volatile members not working as expected

Question

I was reading through the tips and tricks post and I thought I'd try out some of the C# stuff that I'd never done before. Therefore, the following code serves no actual purpose, but is just a 'test function' to see what happens.

Anyway, I have two static private fields:

private static volatile string staticVolatileTestString = "";
[ThreadStatic]
private static int threadInt = 0;

As you can see, I'm testing ThreadStaticAttribute and the volatile keyword.

Anyway, I have a test method that looks like this:

private static string TestThreadStatic() {
    // Firstly I'm creating 10 threads (DEFAULT_TEST_SIZE is 10) and starting them all with an anonymous method
    List<Thread> startedThreads = new List<Thread>();
    for (int i = 0; i < DEFAULT_TEST_SIZE; ++i) {
        Thread t = new Thread(delegate(object o) {
            // The anon method sets a newValue for threadInt and prints the new value to the volatile test string, then waits between 1 and 10 seconds, then prints the value for threadInt to the volatile test string again to confirm that no other thread has changed it
            int newVal = randomNumberGenerator.Next(10, 100);
            staticVolatileTestString += Environment.NewLine + "\tthread " + ((int) o) + " setting threadInt to " + newVal;
            threadInt = newVal;
            Thread.Sleep(randomNumberGenerator.Next(1000, 10000));
            staticVolatileTestString += Environment.NewLine + "\tthread " + ((int) o) + " finished: " + threadInt;
        });
        t.Start(i);
        startedThreads.Add(t);
    }

    foreach (Thread th in startedThreads) th.Join();

    return staticVolatileTestString;
}

What I'd expect to see returned from this function is an output like this:

thread 0 setting threadInt to 88
thread 1 setting threadInt to 97
thread 2 setting threadInt to 11
thread 3 setting threadInt to 84
thread 4 setting threadInt to 67
thread 5 setting threadInt to 46
thread 6 setting threadInt to 94
thread 7 setting threadInt to 60
thread 8 setting threadInt to 11
thread 9 setting threadInt to 81
thread 5 finished: 46
thread 2 finished: 11
thread 4 finished: 67
thread 3 finished: 84
thread 9 finished: 81
thread 6 finished: 94
thread 7 finished: 60
thread 1 finished: 97
thread 8 finished: 11
thread 0 finished: 88

However, what I'm getting is this:

thread 0 setting threadInt to 88
thread 4 setting threadInt to 67
thread 6 setting threadInt to 94
thread 7 setting threadInt to 60
thread 8 setting threadInt to 11
thread 9 setting threadInt to 81
thread 5 finished: 46
thread 2 finished: 11
thread 4 finished: 67
thread 3 finished: 84
thread 9 finished: 81
thread 6 finished: 94
thread 7 finished: 60
thread 1 finished: 97
thread 8 finished: 11
thread 0 finished: 88

The second 'half' the output is as expected (which I suppose means that the ThreadStatic field is working like I thought), but it seems like a few of the initial outputs have been 'skipped' from the first 'half'.

Additionally, the threads in the first 'half' are out of order, but I understand that a thread does not immediately run as soon as you call Start(); but instead the internal OS controls will be starting the threads as it sees fit. EDIT: No they're not, actually, I just thought they were because my brain misses the consecutive numbers

---

So, my question is: What's going wrong to cause me to lose a few lines in the first 'half' of the output? For example, where is the line 'thread 3 setting threadInt to 84'?

Tuff

I run your code a few times and always get the expected output... – Floridafloridia

I'd wager that somewhere down the line, the call to += on the string is getting the value before the previous thread has set it and the new thread is overwriting it with the new value (therefore thread 1 had it's output skipped). – Herndon

@DannyChen I'm on a quad-core, and that's not the only code in that runs in the application; don't know if either of those would make sense. I can upload the entire code somewhere (it's only two .cs files) if you'd like? – Tuff

@Herndon well that's what I thought, and I doubt it's a bug in the framework... – Tuff

This doesnt have to do anything with the quad-core. The same thing can just happen on a single core as well. It depends when exactly the scheduler decides to pause/resume the threads. It may happen more often on a quad-core, but could as well be the other way round... – Parmentier

@Herndon You edited your comment: Yes, I think that's what's happened, Lucero's answer clears it up nicely – Tuff

Yeah I realised that it wasn't what volatile was for, my brain exploded for a second – Herndon

Answer 1

Threads are executing at the same time. What conceptually happens is this:

staticVolatileTestString += Environment.NewLine + "\tthread " + ((int) o) + " setting threadInt to " + newVal;

1. Thread 1 reads staticVolatileTestString
2. Thread 2 reads staticVolatileTestString
3. Thread 3 reads staticVolatileTestString
4. Thread 1 appends the stuff and writes staticVolatileTestString back
5. Thread 2 appends the stuff and writes staticVolatileTestString back
6. Thread 3 appends the stuff and writes staticVolatileTestString back

That causes your lines to be lost. Volatile doesn't help here; the whole process of concatenating the string is not atomic. You need to use a lock around those operations:

private static object sync = new object();

lock (sync) {
    staticVolatileTestString += Environment.NewLine + "\tthread " + ((int) o) + " setting threadInt to " + newVal;
}

Obau

Answer 2

The MSDN describes what the keyword volatile does here:

The volatile keyword indicates that a field might be modified by multiple concurrently executing threads. Fields that are declared volatile are not subject to compiler optimizations that assume access by a single thread. This ensures that the most up-to-date value is present in the field at all times.

This means, in your example, what happens is about this (this can vary from time to time; depends on the scheduler):

• thread 0 is reading the string out of staticVolatileTestString
• thread 0 is appending 'thread 0 setting threadInt to 88'
• thread 0 is writing the string back into staticVolatileTestString

this is like expected so far, but then:

• thread 1-4 are reading the string out of staticVolatileTestString
• thread 1 is appending 'thread 1 setting threadInt to 97'
• thread 2 is appending 'thread 2 setting threadInt to 11'
• thread 2 is writing the string back into staticVolatileTestString
• ... threads 1, 2, 3 are reading and appending, and writing
• thread 4 is writing the string back into staticVolatileTestString
• and so on...

see what happened here? thread 4 read the string 'thread 0 setting threadInt to 88', appended its 'thread 4...' and wrote it back, overwriting everything thread 1, 2 and 3 had written into the string already.

Parmentier

So are you saying that the threads are reading the value simultaneously and then writing as a separate operation all over each other? If that's the case, I'd need some sort of lock on the field (which is what I thought volatile was, but I guess it actually just ensures that there's no caching, right?) EDIT: Lucero's answer confirms this – Tuff

@Motig: The lock is there when reading the string, or writing it back, but not in between. Remember: strings are immutable, therefore a new string instance is created, and the reference to that is then threadsafely written back into the variable. – Parmentier

@Motig: the volatile keyword works great with mutable primitive datatypes (bool, int, double, etc.). With strings however, you want to set a dedicated lock, read the string, modify it and write it back, then release the lock. – Parmentier

@Philip: Be careful. Atomic reads and writes aren't guaranteed for all those types. Section 12.5 of the ECMA C# spec says this: "Reads and writes of other types, including long, ulong, double, and decimal, as well as user-defined types, need not be atomic." If you want guaranteed atomicity for those types then you'll also need a lock, or maybe one of the Interlocked methods. – Unsegregated

@LukeH: thanks for that info; good to know. Also, what I just realized, the same issue is true for a volatile int that is i = i + 1; (read, modify, write). You need a lock there just as well. – Parmentier

@Philip: Yep, or Interlocked.Add. – Unsegregated