0

Why shouldn't I unlock a mutex from a different thread? In the c++ standard it says it pretty clearly: If the mutex is not currently locked by the calling thread, it causes undefined behavior. But as far as I can see, everything works as expected on Linux(Fedora 31 with GCC). I seriously tried everything but I could not get it to behave strangely. All I'm asking for is an example where something, literally anything is affected by unlocking a mutex from a different thread.

Here is a quick test I wrote which is super wrong and shoudn't work but it does:

std::mutex* testEvent;

int main()
{
    testEvent = new std::mutex[1000];

    for(uint32_t i = 0; i < 1000; ++i) testEvent[i].lock();

    std::thread threads[2000];

    auto lock = [](uint32_t index) ->void { testEvent[index].lock(); assert(!testEvent[index].try_lock()); };
    auto unlock = [](uint32_t index) ->void { testEvent[index].unlock(); };

    for(uint32_t j = 0; j < 1000; ++j)
    {
        for(uint32_t i = 0; i < 1000; ++i)
        {
            threads[i]      = std::thread(lock,i);
            threads[i+1000] = std::thread(unlock,i);
        }
        for(uint32_t i = 0; i < 2000; ++i)
        {
            threads[i].join();
        }
        std::cout << j << std::endl;
    }

    delete[] testEvent;
}
A.Hristov
  • 481
  • 1
  • 4
  • 13
  • 3
    *But as far as I can see, everything works as expected on Linux(Fedora 31 with GCC)* The set of results of a undefined program contains the result you are looking for. This is most dangerous part of UB, it can appear to work perfectly. – NathanOliver May 28 '20 at 15:49
  • Can we get a minimal example of your code and we can confirm if you actually have UB or not? – NathanOliver May 28 '20 at 15:50
  • If a tree falls in a forest and no one is around to hear it, does it make a sound? If everything works as expected is anything wrong? – A.Hristov May 28 '20 at 15:51
  • It depends on your code. u can lock it from a thread and then unlock it from another one but u must make sure that it's locked before unlocking it and make sure that it doesn't protect data without a ub. – asmmo May 28 '20 at 15:53
  • 4
    *If everything works as expected is anything wrong?* Yes. You basically have a live bomb. It might not do anything for years, or it might blow up in your face the first time something changes like running on a different machine or compiling with a different compiler. – NathanOliver May 28 '20 at 15:53
  • Well, this part of the code has been around for literally 6 years until I joined the company and found it. – A.Hristov May 28 '20 at 15:54
  • That's not too surprising. There are broken binary searches that have been around for decades. – NathanOliver May 28 '20 at 15:56
  • I've personally be caught by this before. I didn't know at the time that threads couldn't unlock mutex they hadn't locked themselves. It worked fine in Linux until I checked Windows compatibility. Then I realized the system I had written and tested couldn't work the way I intended. I [asked a question on SO](https://stackoverflow.com/questions/44207489/handing-over-locked-stdunique-lock-to-new-threads) about it. Believe me, it just **looks** like it's working for you. – François Andrieux May 28 '20 at 15:56
  • Just because you think it works for you today offers you no guarantees that it will work tomorrow, or when your C or C++ library gets upgraded. – Sam Varshavchik May 28 '20 at 15:58
  • @NathanOliver what about the scenario I've addressed in my previous comment. It may lead to a ub too!! – asmmo May 28 '20 at 15:59
  • @A.Hristov They are still finding buried munitions from WWI. It isn't because your live bomb has been around for a while and hasn't exploded yet that it won't be disturbed in the future and explode. – François Andrieux May 28 '20 at 15:59
  • 1
    UB tends to bite the hardest when you enable the optimizer. And different compilers (or different version of the same compiler) or the same compiler on a different platform may generate different code and behave differently. Even adding or removing unrelated code may cause the compiler to change the behaviour since it's the entire program that is undefined, not just the pieces containing the UB. In short; you just don't have any guarantees about your program behaviour any more and that is *not* a situation you want to be in. It's a ticking bomb that can explode at *any* time with *any* change. – Jesper Juhl May 28 '20 at 15:59
  • @asmmo No, that is unrelated to the problem. In C++, the standard mutex objects can only be unlocked by the thread that locked them. – François Andrieux May 28 '20 at 15:59
  • 1
    Every time I find someone doing this it's because they didn't know they could get what they want with a monitor (condition variable). – user4581301 May 28 '20 at 16:11
  • 1
    Possible duplicate: https://stackoverflow.com/q/62814/963864 – curiousguy Jun 01 '20 at 11:50
  • @JesperJuhl: The problem is that some compiler writers believe that when part of the Standard says something is undefined, that should negate anything else in the Standard or an implementation's documentation that would otherwise specify the behavior. In reality, most such conflicts are intended designed to avoid requiring that compilers uphold corner cases *that aren't important to their customers*, without implying any judgment as to which cases those should be. – supercat Jun 01 '20 at 15:18

2 Answers2

3

As you already said, it is UB. UB means it may work. Or not. Or randomly switch between working and making your computer sing itself a lullaby. (See also "nasal demons".)

Here are just a few ways someone can break your program on Fedora 31 with GCC on x86-64:

  1. Compile with -fsanitize=thread. It will now crash every time, which is still a valid C++ implementation, because UB.
  2. Run unter helgrind (valgrind --tool=helgrind ./a.out). It will crash every time -- still a valid way to host a C++ program, because UB.
  3. The libstdc++/glibc/pthread implementation on the target system switches from using "fast" mutexes by default to "error checking" or "recursive" mutexes (https://manpages.debian.org/jessie/glibc-doc/pthread_mutex_init.3.en.html). Note that this is probably possible in a manner that is ABI-compatible with your program, meaning that it does not even have to be recompiled for it to suddenly stop working.

That being said, since you are using a platform on which the C++ mutex boils down to a futex-implemented "fast" pthread mutex, this does not work by accident. It is just not guaranteed to keep working for any time, or in any circumstance that actually checks if you are doing the right thing.

danielschemmel
  • 10,885
  • 1
  • 36
  • 58
0

I really wonder why you would want to do this in the first place ;)

Normally you would want to have something like

lock();
do_critical_task();
unlock();

(In c++, the lock/unlock is often hidden by use of std::lock_guard or similar.)
Let's assume one thread (lets say Thread A) called this code and is inside the critical task, i.e. it is also holding the lock. Then if you unlock the same mutex from another thread, any thread other than A can also enter the critical section simultaneously.

The main purpose of mutexes is to have mutual exclusion (hence their name), so all you would do is to erase the purpose of the mutex ;)

That said: you should always believe the standard. Only if something works out on a certain system it doesn't mean it's portable. Plus: especially in a concurrent context, a lot of things can work out a thousand times but then fail the 1001'th time as of race conditions. In mathematics your attempt would be comparable to "proof by example".

mrksngl
  • 109
  • 4
  • A mutex lock is a proper type of resource, and sometimes you want to hand over resources between threads. It can be desirable to lock a mutex protecting some data, prepare that data and then pass the lock to another thread that will continue working on the data. The standard doesn't allow it and it requires an extra set of unlock/lock to "transfer" the ownership of the lock. Mutex don't *only* protect critical sections. – François Andrieux May 28 '20 at 16:02
  • Well in the application I'm working on it currently acts as an event. The mutex is locked on construction and then when you try to lock it it waits until somebody unlocks it from another thread. This is super wrong but it has worked for 6 yeas so idk what to think. – A.Hristov May 28 '20 at 16:04
  • @A.Hristov It seems you think it "is super wrong" which is the correct impression to have. – François Andrieux May 28 '20 at 16:13
  • @FrançoisAndrieux: yes you're right, didn't think about that. I even tend to say the C++ committee also had only the critical sections in mind (or at least they didn't want to advertise any other use), as you cannot even transfer the whole mutex by means of `std::move` ;) – mrksngl May 28 '20 at 16:24
  • @A.Hristov: well, wouldn't it be better to use condition variable (with an actual condition to circumvent spurious wakeups) or a semaphore to wait? – mrksngl May 28 '20 at 16:24
  • definitely but by now it's so ingrained that it would take me ages to refactor everything – A.Hristov May 28 '20 at 16:25