pthread_cond_wait strange behaviour

Question

I'm trying to solve Dining philosophers problem using C++.

Code is compiled with g++ -lpthread.

Entire solution is on philosophers github. Repository contains two cpp files: main.cpp and philosopher.cpp. "Main.cpp" creates mutex variable, semaphore, 5 conditinal variables, 5 forks, and starts philosophers. Semaphore is used only to synchronize start of philosophers. Other parameters are passed to philosophers to solve a problem. "Philosopher.cpp" contains solution for given problem but after few steps deadlock occurs.

Deadlock occurs when philosopher 0 is eating, and philosopher 1 (next to him) wants to take forks. Then, philosopher 1 has taken mutex, and wont give it back until philosopher 0 puts his forks down. Philosopher 0 can't put his forks down because of taken mutex, so we have a deadlock. Problem is in Philosopher::take_fork method, call for pthread_cond_wait(a,b) isn't releasing mutex b. Can't figure out why?

// Taking fork. If eather lef or right fork is taken, wait.
void Philosopher::take_fork(){
  pthread_mutex_lock(&mon);
  std::cout << "Philosopher " << id << " is waiting on forks" << std::endl;
  while(!fork[id] || !fork[(id + 1)%N])
    pthread_cond_wait(cond + id, &mon);
  fork[id] = fork[(id + 1)%N] = false;
  std::cout << "Philosopher " << id << " is eating" << std::endl;
  pthread_mutex_unlock(&mon);
}

Please reference to this code for the rest.

Answer 1

Your call to pthread_cond_wait() is fine, so the problem must be elsewhere. You have three bugs that I can see:

Firstly, in main() you are only initialising the first condition variable in the array. You need to initialise all N condition variables:

for(int i = 0; i < N; i++) {
    fork[i] = true;
    pthread_cond_init(&cond[i], NULL);
}

pthread_mutex_init(&mon, NULL);

Secondly, in put_fork() you have an incorrect calculation for one of the condition variables to signal:

pthread_cond_signal(cond + (id-1)%N);    /* incorrect */

When id is equal to zero, (id - 1) % N is equal to -1, so this will try to signal cond - 1, which does not point at a condition variable (it's possible that this pointer actually corrupts your mutex, since it might well be placed directly before cond on the stack). The calculation you actually want is:

pthread_cond_signal(cond + (id + N - 1) % N);

The third bug isn't the cause of your deadlock, but you shouldn't call srand(time(NULL)) every time you call rand() - just call that once, at the start of main().