mutex.try_lock() lets multiple threads hold the lock simultaneously

Question

After hours tearing my hair out, it appears I've been savagely mauled by unexpected behaviour from c++11's unique_lock. I must have horribly misunderstood something :

#include <iostream>
#include <vector>
#include <thread>
#include <mutex>

#define N_THREADS 4
#define N_ITERATIONS 10000
#define N_LOOPS 1000000

class ThingMaJigger
{
public:
    void fight()    {
        if(m.try_lock()) {
            // more then one thread ends up here??!?!?
            printf("I'm the winner!\n" );
            m.unlock();
        } else {
            printf("Sleeping \n" );
        }
    }
private:
    std::mutex m;
};

void worker(ThingMaJigger* ar,int tIdx)
{
    ar->fight();
}

int main(int argc, char const *argv[])
{
    for (int _loop = 0; _loop < N_LOOPS; ++_loop) {
        std::vector<std::thread> ts;
        ThingMaJigger t;
        for (int i = 0; i < N_THREADS; ++i)
        {
            ts.emplace_back(worker,&t,i);
        }

        for (int i = 0; i < N_THREADS; ++i)
        {
            ts[i].join();
        }

        printf("\n");

    }
        return 0;
}

Compile with clang++ -std=c++11 -O2 -lpthread ./unique_lock.cpp

clang 3.7.0, g++ 5.1.1, both behave in the same way.

Example output:

I'm the winner!
Sleeping 
Sleeping 
I'm the winner!

I'm the winner!
Sleeping 
I'm the winner!
Sleeping 

I'm the winner!
I'm the winner!
Sleeping 
Sleeping 

Kinda looks like multiple workers holding the same lock at the same time, don't it?

http://en.cppreference.com/w/cpp/thread/mutex/try_lock says:

Return value

true if the lock was acquired successfully, otherwise false.

Note: try_lock is allowed to return false even if no one else has the lock. That's not what this is about.

Answer 1

This is working as intended.

Immediately after printing "I'm the winner," you're unlocking the lock. This gives the other threads a chance to acquire it as well.

If you want only one thread to "win," you should also have a variable indicating whether someone has won yet. Set it to false before creating the threads. Any thread which acquires the lock sucessfully checks that variable to see if anyone else has won.

bool somebodyWon = false; // make sure this is set to false before
                          // threads get created

    if(m.try_lock()) {
        if (somebodyWon) {
            printf("Darn, someone beat me to it!\n");
        } else {
            printf("I'm the winner!\n");
            somebodyWon = true;
        }
        m.unlock();
    } else {
        printf("I didn't even get a chance! \n" );
    }

The other approach, which is also legal, is to change from using a mutex to a semaphore so that one thread can lock the object, but let the parent thread release it after all threads have joined.

Answer 2

Conccurency is confusing. Let's go shopping!

#include <iostream>
#include <vector>
#include <thread>
#include <mutex>
#include <unistd.h>

#define N_THREADS 4
#define N_ITERATIONS 10000
#define N_LOOPS 1000000

class ThingMaJigger
{
public:
    void fight()    {
        if(m.try_lock()) {
            // more then one thread ends up here??!?!?
            printf("I'm the winner!\n");
            usleep(1000000); // <<<< this. 
                             // or can unlock() before other's try_lock()
            m.unlock();
        } else {
            printf("Sleeping \n" );
        }
    }
private:
    std::mutex m;
};

void worker(ThingMaJigger* ar,int tIdx)
{
    ar->fight();
}

int main(int argc, char const *argv[])
{
    for (int _loop = 0; _loop < N_LOOPS; ++_loop) {
        std::vector<std::thread> ts;
        ThingMaJigger t;
        for (int i = 0; i < N_THREADS; ++i)
        {
            ts.emplace_back(worker,&t,i);
        }

        for (int i = 0; i < N_THREADS; ++i)
        {
            ts[i].join();
        }

        printf("\n");

    }
        return 0;
}