38

With GCC 4.8.2 (on Linux/Debian/Sid 64 bits) -or GCC 4.9 when available - in C++11- I have some mutex

std::mutex gmtx;

actually, it is a static member in some class Foo containing both alpha and beta methods below.

it is locked in alpha like

void alpha(void) {
   std::lock_guard<std::mutex> g(gmtx);
   beta(void);
   // some other work
}

and I want to check in beta that indeed gmtx is locked:

void beta(void) {
   assert (gmtx.is_locked());
   // some real work
}

(notice that is_locked is only called inside assert... It can be very inefficient or even sometimes inaccurate)

Of course, I have other functions calling beta, e.g.

void gamma(void) {
   std::lock_guard<std::mutex> g(gmtx);
   beta();
   // some other work
}

but is_locked does not exist.... How should I define it? (actually I would like to be sure that the mutex has been locked in the same thread by some [indirect] caller...)

(the reason I want to test that with assert is that beta could be called elsewhere)

I cannot use try_lock (unless using recursive mutexes), because in the common case it would lock an already locked mutex... (locked in the same thread by a caller) and this is not only undefined behavior but blocks entirely.

I want to avoid recursive mutexes (more costly than plain mutexes) unless I really have to.


NB: The real program is a bit more complex. Actually, all the methods are inside a class which maintain a naming bi-directional relation on "items". So I have inside that class a map from items to names and another from names to items. beta would be the internal method adding really a naming, and alpha and gamma would be the methods finding -or adding- an item by its name, or a name by its item.

PS: the real program is not yet released, but should become part of MELT - its future monitor; you can download it (alpha stage, very buggy) from here (a temporary location)

Basile Starynkevitch
  • 223,805
  • 18
  • 296
  • 547
  • if `beta` requires that the mutex is always locked (as your `assert` suggests) why are you pushing that responsibility up the call chain? – John Ledbetter Feb 19 '14 at 21:47
  • Because (as I edited) some other function like `gamma` would call `beta` ... – Basile Starynkevitch Feb 19 '14 at 21:51
  • 1
    Right I see, but both are required to lock the mutex before calling beta, so why is beta not just locking the mutex? Are you saying that both callers have other work to do that requires the mutex be locked? – John Ledbetter Feb 19 '14 at 21:52
  • @JohnLedbetter: Yes, both callers do real work. – Basile Starynkevitch Feb 19 '14 at 21:53
  • I have the same use case here. Strange that there is an owns_lock on the lock class but not on the mutex. I can spend a question on Boost forum. – gast128 Oct 12 '15 at 08:46
  • 2
    Wrap the lock in a class that maintains a 'locked' boolean variable and check that? – rubenvb Jan 13 '17 at 06:46
  • @gast128 The C++ mutex is designed to be an absolute minimum implementation to map efficiently to low level / OS level constructs. There's an overhead to tracking which thread owns the lock and there's a correct assumption that if we're using concurrency the aim is to strictly (aggressively) minimise overheads. Most good designs never need to check if they own a mutex. The control flow is implemented to ensure correct behaviour. You can add it to your own 'lockable' wrapper but it wouldn't be possible to subtract it. – Persixty Dec 02 '20 at 02:24

7 Answers7

23

Strictly speaking, the question was about checking the lockedness of std::mutex directly. However, if encapsulating it in a new class is allowed, it's very easy to do so:

class mutex :
    public std::mutex
{
public:
#ifndef NDEBUG
    void lock()
    {
        std::mutex::lock();
        m_holder = std::this_thread::get_id(); 
    }
#endif // #ifndef NDEBUG

#ifndef NDEBUG
    void unlock()
    {
        m_holder = std::thread::id();
        std::mutex::unlock();
    }
#endif // #ifndef NDEBUG

#ifndef NDEBUG
    bool try_lock()
    {
        if (std::mutex::try_lock()) {
            m_holder = std::thread::id();
            return true;
        }
        return false;
    }
#endif // #ifndef NDEBUG

#ifndef NDEBUG
    /**
    * @return true iff the mutex is locked by the caller of this method. */
    bool locked_by_caller() const
    {
        return m_holder == std::this_thread::get_id();
    }
#endif // #ifndef NDEBUG

private:
#ifndef NDEBUG
    std::atomic<std::thread::id> m_holder = std::thread::id{};
#endif // #ifndef NDEBUG
};

Note the following:

  1. In release mode, this has zero overhead over std::mutex except possibly for construction/destruction (which is a non-issue for mutex objects).
  2. The m_holder member is only accessed between taking the mutex and releasing it. Thus the mutex itself serves as the mutex of m_holder. With very weak assumptions on the type std::thread::id, locked_by_caller will work correctly.
  3. Other standard library types like std::lock_guard are templates, so they work well with this new class, because it satisfies the Mutex requirement.
Jan Schultke
  • 17,446
  • 6
  • 47
  • 96
Ami Tavory
  • 74,578
  • 11
  • 141
  • 185
  • 3
    for `locked_by_caller` to be of any use you have to be able to call it even, when you haven't locked the mutex yourself. so `m_holder` needs to be an atomic for this code to be legal. – MikeMB May 22 '18 at 15:20
  • I think this is the best solution and I use something like this too. Any reason why didn't update `m_holder` type to atomic as suggested? – user1593842 Jul 16 '19 at 21:43
  • This is rife with race conditions. Because the lock/unlock is not performed atomically with the setting of the locker. – Wheezil Aug 05 '20 at 21:16
  • 1
    I revise my comment on further thought. This has some race conditions, because the lock/unlock is not performed atomically with the setting of the locker. Is does reliably answer the question "does THIS thread have the mutex locked", which is probably what the OP wanted. It doesn't reliably answer the question "does some other thread have the mutex locked" – Wheezil Aug 05 '20 at 21:29
18

std::unique_lock<L> has owns_lock member function (equivalent of is_locked as you say).

std::mutex gmtx;
std::unique_lock<std::mutex> glock(gmtx, std::defer_lock);

void alpha(void) {
   std::lock_guard<decltype(glock)> g(glock);
   beta(void);
   // some other work
}
void beta(void) {
   assert(glock.owns_lock()); // or just assert(glock);
   // some real work
}

EDIT: In this solution, all lock operations should be performed via unique_lock glock not 'raw' mutex gmtx. For example, alpha member function is rewritten with lock_guard<unique_lock<mutex>> (or simply lock_guard<decltype(glock)>).

yohjp
  • 2,985
  • 3
  • 30
  • 100
  • 9
    Concurrently calling `lock()`/`unlock()` on a `unique_lock` from different threads results in a data race, which makes this rather useless. – T.C. Feb 11 '16 at 00:57
  • 7
    `std::unique_lock` is not thread safe and is not meant to be used in multiple threads. The implementation I have seen stores a bool value of whether it owns the lock solely based on `lock()` and `unlock()` calls, independent of the calling thread. Therefore, locking a global `std::unique_lock` in one thread will make `owns_lock()` return true for **every** thread, which is very incorrect behavior. – joshwilsonvu Aug 03 '17 at 19:11
  • This isn't really workable. Typically the lock guard is on the stack in some method, not a member of the class, and there may be many lock guards in many threads. I think what the OP really wants to know is "does THIS thread have the mutex locked?" – Wheezil Aug 05 '20 at 21:15
14

You could just use a recursive_mutex, which can be locked multiple times on the same thread. Note: If it were my code, I would restructure it so that I don't need a recursive_mutex, but it will address your problem.

Nevin
  • 4,595
  • 18
  • 24
  • 6
    @BasileStarynkevitch: look at it this way -- if a mutex can detect whether or not it is locked by this thread, then it can already implement recursive locking with no further significant time overhead. Therefore, a hypothetical mutex with the property you desire will not slow the code down any less than using a recursive mutex. – Steve Jessop Feb 19 '14 at 21:54
  • A caution with recursive_mutex is that it cannot be used as the lock for condition_variable but other than that it is a good answer. It doesn't directly answer the OP's question -- checking if my thread already owns the lock, but it presumably avoids the bad condition (altering data structures without holding the lock). – Wheezil Aug 05 '20 at 21:26
2

Well, if the expense of the assertion really isn't an issue then you can just call try_lock() from another thread where its behavior is guaranteed to be well defined:

void beta(void) {
  assert(std::async(std::launch::async, [] { return gmtx.try_lock(); })
                 .get() == false &&
         "error, beta called without locking gmtx");
  // some real work
}
bames53
  • 86,085
  • 15
  • 179
  • 244
2

Try atomic (e.g. atomic<bool> or atomic<int>), which has a nice load function that will do what you want, as well as other nice functions like compare_exchange_strong.

lmat - Reinstate Monica
  • 7,289
  • 6
  • 48
  • 62
Andrew
  • 5,839
  • 1
  • 51
  • 72
  • 1
    Wow, just ended two hours of research and found that I had upvoted this answer long ago...coming to the same conclusion. Although, I'm beginning to think that extending `std::mutex` might be a good way to go...ugh why isn't this functionality on `std::mutex` already? – lmat - Reinstate Monica May 18 '18 at 19:28
  • 1
    @LimitedAtonement I had found in my experience that atomic was easiest and most straightforward to use. – Andrew May 18 '18 at 19:33
  • @LimitedAtonement Also, if you want to see an example of this in action, I have another answer (that someone just upvoted so I just remembered it) that puts atomic into good use, for asynchronous Console input: https://stackoverflow.com/a/42264216/1599699 – Andrew May 23 '18 at 02:06
  • This doesn't really answer the question, but I also find that using atomics can help solve this class of problems. – Wheezil Aug 05 '20 at 21:23
  • @Wheezil From what I remember, the `atomic` class could basically satisfy any multi-threading challenges one would have. Also, according to my answer, the `load()` function will actually do exactly what the OP wants, which I believe is true. – Andrew Aug 06 '20 at 03:21
  • @Wheezil `while (atomicBool) {}`? – Andrew Aug 07 '20 at 17:15
  • 1
    @Andrew Yes, of course! Spin lock. That will work just fine so long as your wait time is expected to be small (such as guarding a data structure) or there are lots of cores to burn. I should have said "won't block the thread and free up the CPU resource". IIRC correctly, a lot of mutex implementations do a bit of spin before asking the OS to intervene. – Wheezil Aug 08 '20 at 22:53
2

It's not technically an assertion, but I've used a similar approach to prevent unlocked access to shared state: add a reference parameter to the lock guard class on the in the unsafe function (beta in your example). Then the function cannot be called unless the caller has created a lock guard. It solves the problem of accidentally calling the function outside of the lock, and it does so at compile time with no races.

So, using your example:

typedef std::lock_guard<std::mutex> LockGuard;
void alpha(void) {
   LockGuard g(gmtx);
   beta(g);
   // some other work
}

void beta(LockGuard&) {
   // some real work
}

void gamma(void) {
   LockGuard g(gmtx);
   beta(g);
   // some other work
}

//works recursively too
void delta(LockGuard& g)
{
   beta(g);
}

Drawbacks:

  • Does not validate that the lock actually wraps the correct mutex.
  • Requires the dummy parameter. In practice, I usually keep such unsafe functions private so this is not an issue.
M Carrato
  • 21
  • 2
-3

My solution is simple, use try_lock to test then unlock if needed:

std::mutex mtx;

bool is_locked() {
   if (mtx.try_lock()) {
      mtx.unlock();
      return false;
   }
   return true; // locked thus try_lock failed
}
Tony
  • 1,551
  • 20
  • 21
  • 2
    This does not work. try_lock on std::mutex when it is already locked by THIS thread is undefined and typically throws std::system_exception – Wheezil Aug 05 '20 at 21:22