4

I know my question is quite similar to this Why does std::lock_guard release the lock after using std::adopt_lock?, but the behavior I see is not. Here is my code:

#include <mutex>
#include <iostream>
using namespace std;

std::mutex m;
void fun2();
void fun1() {
    cout << "fun1" << endl;
    std::unique_lock<std::mutex> guard(m);
    cout << "lock mutex" << endl;
    fun2();
    if (guard.owns_lock()) {
        cout << "still holds mutex" << endl;
    }
    else {
        cout << "doesn't hold mutex" << endl;
    }
}
void fun2() {
    std::lock_guard<std::mutex> guard(m, std::adopt_lock);
    cout << "fun2" << endl;
}
int main(int argc, char* argv[]) {
    fun1();
    return 0;
}

And this is the result I get:

fun1
lock mutex
fun2
still holds mutex

Clearly, the unique_lock in fun1 still holds the mutex. So my question is "Does std::lock_guard release the mutex after constructed with std::adopt_lock option?". Hope you all can help me clarify this situation. Thank you.

Huy Nhat Tran
  • 135
  • 11
  • `std::adopt_lock` locking strategy assumes the thread already has ownership of the mutex. – Harry Oct 14 '21 at 04:39
  • But will `std::lock_guard` unlock the mutex in the destructor when go out of `fun2`? – Huy Nhat Tran Oct 14 '21 at 04:43
  • yes it will, but the `guard` inside `fun1` will not come to know about the mutex state change. You can check the mutex state change by calling `if (m.try_lock()) cout << "locked\n";` inside `fun1`, after calling `fun2` – Harry Oct 14 '21 at 05:13
  • @TedLyngmo if that is the case how did the `try_lock` inside `fun1` succeed in my previous comment code? – Harry Oct 14 '21 at 05:19
  • @Harry Did you try `try_lock` even before calling `fun2`? You are not `try_lock`ing the right thing. `if (guard.try_lock()) cout << "locked\n";` would be the proper `try_lock` – Ted Lyngmo Oct 14 '21 at 05:21
  • I have tried to use `try_lock` and it will throw exception even before or after `fun2`. Btw, this is the official doc of `owns_lock`: "Checks whether *this owns **a locked mutex** or not." – Huy Nhat Tran Oct 14 '21 at 05:24
  • @TedLyngmo sorry about that, it's undefined behavior calling try_lock by the thread that already owns it. – Harry Oct 14 '21 at 05:29
  • @Harry No, it's not UB. It should throw a `std::system_error` - and it will, no matter if you do it before or after calling `fun2` – Ted Lyngmo Oct 14 '21 at 05:31
  • @TedLyngmo check this link https://en.cppreference.com/w/cpp/thread/mutex/try_lock – Harry Oct 14 '21 at 05:34
  • @Harry Again, you are looking at the wrong `try_lock`. It's the _guard's_ `try_lock` you should use. https://en.cppreference.com/w/cpp/thread/unique_lock/try_lock - so `if (guard.try_lock()) cout << "locked\n";` before or after calling `fun2` will throw. – Ted Lyngmo Oct 14 '21 at 05:35
  • Hmm... I can't figure out how to make this into something that doesn't have UB. I made my own mutex to check if the guards are doing anything to solve it but [it appears](https://godbolt.org/z/YeTGxoWWs) they are not. – Ted Lyngmo Oct 14 '21 at 06:31
  • `guard` in `fun2` releases the mutex on destruction, however it does not magically report it to any other component, so `guard` in `fun1` still thinks it owns the lock. – n. m. could be an AI Oct 14 '21 at 06:36
  • @n.1.8e9-where's-my-sharem. Yes - and it will unlock it - causing UB - so I don't think using a guard and then adopt the mutex by another guard in `fun2` is correct. `m.lock()` (without a guard) in `fun1` would be the appropriate action I think. – Ted Lyngmo Oct 14 '21 at 06:36

2 Answers2

2

When you constructed a std::unique_lock to manage the mutex, you should stick to it unless you first break the association of the std::unique_lock with the mutex using std::unique_lock::release. In your sample, you touched the raw mutex when it's still managed by a std::unique_lock and this is wrong.

Lingxi
  • 14,579
  • 2
  • 37
  • 93
  • Oh I see now. I have read the implementation of `std::unique_lock` and found out that `std::unique_lock::owns_lock` return the previous state of mutex (which is `true` when I call `std::unique_lock guard(m)`). It doesn't know the mutex has been unlocked when go out of `fun2`. Thank you ^^ – Huy Nhat Tran Oct 14 '21 at 07:11
1

The program has undefined behavior.

You've created two guards that both think they own the mutex and they will both unlock it. That's UB.

Using m.lock() in fun1 instead of using a guard would be one way to make it have defined behavior.

Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
  • 1
    This is the implementation of destructor of `std::lock_guard`: `~lock_guard(){ _M_device.unlock(); }`. As far as I can see, `lock_guard` always call unlock method of internal reference mutex `_M_device`. So it will not check the condition as you said – Huy Nhat Tran Oct 14 '21 at 05:41
  • @HuyNhatTran I guess one would have to dig deeper and look at what `_M_device.unlock()` does too. I don't have that infront of me. I'll delete this for now though. You got me unsure :-) – Ted Lyngmo Oct 14 '21 at 05:43
  • The C++20 standard says unequivocally that the destructor `~lock_guard()` shall (as if) call `pm.unlock()`. There's no exception made for the `adopt_lock` case. See thread.lock.guard p7. – Nate Eldredge Oct 14 '21 at 05:44
  • @NateEldredge Indeed. I updated the answer. – Ted Lyngmo Oct 14 '21 at 06:51