Simplest way to make a whole method thread-safe?

Question

There seems to be a lot to learn about multithreaded programming and it's all a bit intimidating.

For my current needs, I just want to protect against a method being called again from another thread before it finishes, and my question is:

Is this an adequate (safe) way to make a method thread-safe?

class Foo
{
    bool doingWork;
    void DoWork()
    {
        if (doingWork)  // <- sophistocated thread-safety
            return;     // <-

        doingWork = true;

        try
        {
            [do work here]
        }
        finally
        {
            doingWork = false;
        }
    }
}

If that isn't sufficient, what is the simplest way to achieve this?

---

EDIT: More info about the scenario:

• There is only one instance of Foo

• Foo.DoWork() will be called from a ThreadPool thread on the Elapsed event of a System.Timers.Timer.

• Normally Foo.DoWork() will finish eons before the next time it's called, but I want to code for the slim chance that it will run long, and get called again before finishing.

---

(I'm also not smart enough to be sure if this question could be tagged language-agnostic, so I haven't. Enlightened readers, feel free to do so if applicable.)

Answer 1

Your code is not thread safe. You should use the lock keyword instead.

In your current code:

  if (doingWork)
        return;

  // A thread having entered the function was suspended here by the scheduler.

  doingWork = true;

When the next thread comes through, it will also enter the function.

This is why the lock construct should be used. It basically does the same as your code, but without the risk for a thread being interrupted in the middle:

class Foo
{
    object lockObject = new object;
    void DoWork()
    {
        lock(lockObject)
        {
            [do work here]
        }
    }
}

Note that this code has somewhat different semantics than your original. This code will cause the second thread entering to wait and then do the work. Your original code made the second thread just abort. To come closer to your original code, the C# lock statement cannot be used. The underlying Monitor construct has to be used directly:

class Foo
{
    object lockObject = new object;
    void DoWork()
    {
        if(Monitor.TryEnter(lockObject))
        {
            try
            {
                [do work here]
            }
            finally
            {
                Monitor.Exit(lockObject);
            }
        }
    }
}