CODEWITHSUNDEEP
c#thread-safety
Mark
Mark

Reputation: 11740

Is it safe to access this variable from multiple threads?

I commonly write code that is along these lines:

public class MyClass
{
    static bool m_stop = false;

    public static void Main()
    {
        var th = new Thread(DoStuff);

        th.Start();

        Console.ReadLine();

        m_stop = true;

        th.Join();
    }

    private static void DoStuff()
    {
        while( !m_stop )
        {
            // do things here
        }
    }
}

... and I always synchronize access to the variable m_stop, just out of habit. I'm wondering whether it's actually necessary to do that (assuming running an extra loop or two in DoStuff() won't hurt anything). What is the worst that could happen if the code executed exactly like this? I've toyed with the thought of using the volatile keyword for m_stop, but I'm not sure that even that is necessary. Anyone know for sure?

Upvotes: 1

Views: 952

Answers (3)

Jon Skeet
Jon Skeet

Reputation: 1500595

Without being volatile and without any synchronization, the code that you've shown isn't thread-safe. One thread could change the value of the variable and the other thread might never see it, looping forever. If you synchronize all access to it with a common lock, it is thread-safe.

Now volatility is a tricky topic - I thought I understood it until recently, but it doesn't mean quite what I thought it did. However, I'm pretty sure that just making the variable volatile will do what you need it to.

For anything more complicated, I'd usually use full synchronization: take out a lock every time you access the shared data.

Upvotes: 4

unknown
unknown

Reputation:

you must place a memory barrier when you update m_stop. so you will have to mark it as volatile (or place memory barriers/volatile reads-writes yourself)

Upvotes: 0

Andrew Hare
Andrew Hare

Reputation: 351516

While there is nothing technically wrong with this approach you can get into trouble when your application needs to scale. Also you don't need to make m_stop volatile if you are synchronizing access to it.

Upvotes: 1

Related Questions