Search code examples
c++multithreadingpthreadsmutexstdthread

Is there any case where it's valid to unlock then lock an unlocked mutex, while another thread tries to lock it with a lock_guard?


I apologize for the poor title, just looking for someone to confirm Im not crazy.

I recently stumbled across some code that has been in use for years without anyone complaining. The reason I was looking into it was because I was getting this error while heavily modifying this codebase:

pthread_mutex_lock.c:90: ___pthread_mutex_lock: Assertion `mutex->__data.__owner == 0' failed.

After some digging, I found a pretty weird (to me) use of a mutex. In essence:

  1. Two threads are spawned and detached from the main thread (Both threads are in the same .cpp file, and share a globally declared mutex)
  2. In the one that seems to be hit first, the mutex is unlocked and then locked with a 10us pause in a loop, to "allow the other thread to process messages" according to a comment
  3. The thread that would likely get hit second (spawned first, but waits to receive data) has a case in a switch statement that locks the mutex with a lock_guard

I believe this is undefined behavior in multiple ways, but I am getting some pushback in saying that it should be changed as I am struggling to reproduce this exact error consistently and minimally.

I assume this is intended to operate such that the mutex is unlocked first by the looping thread (from an unlocked state, no data for the lock_guard thread to process yet), then locked, and would spend most of its time locked, while the lock_guard occasionally tries to lock, and has to wait for the loop to unlock it, at which point it can process its data, lock_guard goes out of scope, and the lock is returned to the looping thread.

What I think is causing my error is; The lock_guard thread gets data sooner than expected with my modifications, and so the lock_guard case is triggered, and then the looping thread tries to unlock another threads mutex.

I am looking for:

  • Confirmation that it is UB to declare a mutex, then unlock it without it being locked
  • Confirmation that it is UB if the looping thread unlocks the mutex while it is held by the lock_guard
  • Confirmation of my assumption of what is causing my error (I basically already know the first two from references, but want to be sure Im not missing some big brain way of doing things I am unaware of)
  • Maybe a better way to do this, I would assume this could be fixed just by locking the mutex outside of the loop first?

I have searched the codebase and the mutex is only used 4 times that I can see: When it is declared, when it is used in the lock_guard and when it is unlocked, then locked so my while my MRE is short, I think it basically has everything it needs. to demonstrate how this mutex is being used:

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

std::mutex data_mtx;

void lg_thread(){
  std::lock_guard<std::mutex> guard(data_mtx);

  usleep(10e6);
}

int main(int argc, char const* argv[]){
  std::thread t1(lg_thread);

  usleep(10000);

  for (int i = 0; i < 100; i++){
    data_mtx.unlock();
    usleep(500);
    data_mtx.lock();
  }

  return 0;
}

Solution

  • I am looking for:

    • Confirmation that it is UB to declare a mutex, then unlock it without it being locked

    Unlocking a mutex that is not currently locked/owned by the calling thread is undefined behavior, yes.

    • Confirmation that it is UB if the looping thread unlocks the mutex while it is held by the lock_guard

    Yes, it is UB. See above.

    • Confirmation of my assumption of what is causing my error (I basically already know the first two from references, but want to be sure Im not missing some big brain way of doing things I am unaware of)

    See above.

    • Maybe a better way to do this

    Consider using a std::condition_variable instead to coordinate the threads.

    I would assume this could be fixed just by locking the mutex outside of the loop first?

    Yes. That way, if main() locks the mutex first then the lock_guard will block waiting for main() to unlock it, and if the lock_guard locks the mutex first then main() will block waiting for the lock_guard to unlock it. As it should be.