Simplest Mutex ever. Does this example work? Is it thread-safe?

Question

I would like to ask about the simplest Mutex approach ever for multi-threading. Is the following code thread-safe (quick-n-dirty)?

class myclass
{
    bool locked;
    vector<double> vals;

    myclass();
    void add(double val);
};

void myclass::add(double val)
{
    if(!locked)
    {
        this->locked = 1;
        this->vals.push_back(val);
        this->locked = 0;
    }
    else
    {
        this->add(val);
    }
}

int main()
{
    myclass cls;
    //start parallelism
    cls.add(static_cast<double>(rand()));
}

Does this work? Is it thread-safe? I'm just trying to understand how the simplest mutex can be written.

If you have any advice about my example, would be nice.

Thank you.

Thanks for saying that it doesn't work. Can you please suggest a fix which is compiler independent?

Answer 1

Is it thread-safe?

Certainly not. If a thread is preempted between checking and setting the lock, then a second thread could acquire that lock; if control then returns to the first thread, then both will acquire it. (And of course, on a modern processor, two or more cores could be executing the same instructions simultaneously for even more hilarity.)

At the very least, you need an atomic test-and-set operation to implement a lock like this. The C++11 library provides such a thing:

std::atomic_flag locked;

if (!locked.test_and_set()) {
    vals.push_back(val);
    locked.clear();
} else {
    // I don't know exactly what to do here; 
    // but recursively calling add() is a very bad idea.
}

or better yet:

std::mutex mutex;

std::lock_guard<std::mutex> lock(mutex);
vals.push_back(val);

If you have an older implementation, then you'll have to rely on whatever extensions/libraries are available to you, as there was nothing helpful in the language or standard library back then.

Answer 2

No, this is not thread safe. There's a race between

if(!locked)

and

this->locked = 1;

If there is a context switch between these two statements, your lock mechanism falls apart. You need an atomic test and set instruction, or simply use an existing mutex.