0

I know what the following code does and I know why it is a broken code for synchronization as it has only one conditional variable while we need two but I don't know how to provide a sequence of interleaving threads for showing it doesn't work. Can you show why this code doesn't work with an example?

1   cond_t cond = PTHREAD_COND_INITIALIZER; 
2  mutex_t mutex=PTHREAD_MUTEX_INITIALIZER;;
3
4  void *producer(void *arg) {
5    int i;
6    for (i = 0; i < loops; i++) {
7      Pthread_mutex_lock(&mutex);
8      while (count == 1)
9        Pthread_cond_wait(&cond, &mutex);
10     put(i);
11     Pthread_cond_signal(&cond);
12     Pthread_mutex_unlock(&mutex);
13   }
14 }
15
16 void *consumer(void *arg) {
17   int i;
18   for (i = 0; i < loops; i++) {
19     Pthread_mutex_lock(&mutex);
20     while (count == 0)
21       Pthread_cond_wait(&cond, &mutex);
22     int tmp = get();
23     Pthread_cond_signal(&cond);
24     Pthread_mutex_unlock(&mutex);
25     printf("%d\n", tmp);
26   }
27 }
Mona Jalal
  • 34,860
  • 64
  • 239
  • 408

2 Answers2

1

"cond" is not initialized. You have a race condition between consumer and producer. If any of the "while" condition becomes true, they will result in infinite loop as no instruction changes the values of "count" in the "while" scopes (both).

Sebastien
  • 1,439
  • 14
  • 27
  • @Sebastian: I have edited the code. In this code can you provide a sequence of consumer/producer thread that will break the code? – Mona Jalal Oct 28 '13 at 19:53
  • Could it be that the "while" should be refering to "cond" rather than "count"? Also it's difficult to read without the function that initiate the threads. – Sebastien Oct 28 '13 at 20:18
  • The while is correct while this code won't work as you need one conditional variable for empty and on for fill! but I am not asking for fixing the code actually. I need a sequence of consumer/producer threads which can break this code. – Mona Jalal Oct 28 '13 at 20:21
  • Well count do not have initial values. There's many things wrong with this code. So I can't tell you about the part that you don't know are wrong and why if I can't access those you know are wrong. So fix everything you can fix and we'll try to tackle what's remaining... And that requires the thread generating code... – Sebastien Oct 28 '13 at 20:30
1

Presuming that put() sets count to 1 and get() sets count to 0, this code is actually fine as long as you have only one producer and one consumer.

If you have more than one producer, then the pthread_cond_signal() in the producer might wake up one of the other producers instead of a consumer, and then no consumers will proceed. The same problem exists if you have more than one consumer.

You can fix this either by introducing separate condition variables for empty and full, or by using pthread_cond_broadcast() instead of pthread_cond_signal().

caf
  • 233,326
  • 40
  • 323
  • 462