0

I have a thread that goes in a loop doing something like this

void thread()
{
  pthread_mutex_lock(mutex);

  for ( ; ; )
  {
    /** code **/
    {
      pthread_mutex_unlock(mutex);
      /** do_something **/
      pthread_mutex_lock(mutex);
    }

    if (exit)
      return;
  }
}

where exit will be updated in a different thread to true if needed. My problem is that this works fine when do_something takes some time, but if it doesn't the other thread never gets the mutex back. Or it takes too long to do so.

What is the correct pattern in a situation like this?

cauchi
  • 1,192
  • 2
  • 13
  • 22
  • I'm curious why you aren't using `std::thread` pthreads are awful and don't work across platforms – Aluan Haddad Nov 13 '17 at 08:47
  • 2
    its a requirenment of the system I'm working on. std::thread are available on c++11 and I cannot assume that the compiler is c++11 compatible. Let's just say its not up to me. Otherwise yes, I would go with std::thread. – cauchi Nov 13 '17 at 08:52

2 Answers2

4

In this function, you lock mutex and keep it locked except for a tiny part, where you do_something, letting the other threads aquire the mutex, before waiting to re-lock it.

What's wrong ?

  1. When exit is true, the thread finishes, leaving mutex locked and blocking all the other threads waiting to acquire it. Maybe it's on purpose, but for me, it looks rather suspicious (especially if you have some pthread_join() somewhere).

  2. The function seems not exception safe either, for a similar reason: should an exception occur in /* code */, would it cause mutex to remain locked, and your process stuck forever due to the other threads waiting for mutex. std::thread offers lock_guard as a wrapper that makes sure according to the RAII principle that the lock is removed if the guard would be destroyed for example due to stack unwinding.

  3. Depending on do_something, the other threads waiting to acquire mutex might starve (e.g. no other thread get an opportunity to acquire the lock, or always the same get the lock at the expense of all the others, etc...).

How to get it right ?

It's not easy with the litle we know to give you the best advice. However, at first sight, it seems that you could think of using another synchronization mechanism, the condition variables. These exist as std::condition_variable if you use the standard threads. But they exist also for pthreads, as explained in detail in this article.

Long story short, condition variables do something similar to your code: several threads are waiting with pthread_cond_wait() for an event that is advertised using a condition variable. A thread wakes up the waiting threads with pthread_cond_broadcast() or pthread_cond_signal(). One of the thread gets control with the locked mutex.

Christophe
  • 74,672
  • 10
  • 115
  • 187
4

The basic problem here seems to be that this thread keeps the mutex locked by default, and only unlocks it for a limited period of time when it's sure it doesn't need to the mutex.

Normally, you want to do roughly the opposite: the mutex is unlocked by default, and only gets locked for a short period of time when absolutely necessary. Without knowing at least a little bit about the rest of the code, it's hard to be sure of the right way to do that though.

One possibility would be restructure the code so that only a single thread has to deal directly with the shared resources, and the other two threads each have a queue of requests they submit to that thread. That thread can then prioritize the requests, so both client threads get an equal chance to do their thing with the shared resources (or maybe not necessarily equal, but an appropriate amount for each).

As an aside, if you can't use C++11 threading classes, I'd at least consider writing rough analogs of them--a mutex class that calls pthread_mutex_create in its ctor, pthread_mutex_destroy in its dtor, and has a lock and unlock that call pthread_mutex_lock and pthread_mutex_unlock respectively.

Likewise, a lock_guard that's passed a mutex that it locks in its ctor, and unlocks in its dtor (and possibly a similar wrapper for condition variables, etc.) Bottom line: if you're starting from pthreads, you can write a pretty substantial subset of the standard library threading pretty quickly and easily.

Both of these are pretty trivial, and simplify the rest of the code enough that the time to develop them will be repaid quite quickly. In addition, they'll get you things like exception safety that are much more difficult to implement in most other ways.

Jerry Coffin
  • 44,385
  • 5
  • 89
  • 162
  • Thanks. Yes, I have access to all C++11 functionality. I left that part out of the question because it did not seem important. The code is large, and it already took me quite a while to find this block which actually seldon happens. I was just out of ideas about how to approach the issue. This made my brain click: "Normally, you want to do roughly the opposite: the mutex is unlocked by default, and only gets locked for a short period of time when absolutely necessary." – cauchi Nov 14 '17 at 08:26