CON38-C. Notify all threads waiting on a condition variable instead of a single thread

When a given thread waits (cnd_wait() or cnd_timedwait()) on a condition variable, it can be awakened as a result of a signal operation (cnd_signal()). However, if multiple threads are waiting on the same condition variable, any of those threads can be picked up by the scheduler to be awakened (assuming that all threads have the same priority level).

The user is forced to create a predicate-testing loop around the wait condition to guarantee that each thread executes only if its predicate test is true (recommendation in IEEE Std 1003.1 since the 2001 release [IEEE Std 1003.1-2004]). As a consequence, if a given thread finds the predicate test to be false, it waits again, eventually resulting in a deadlock situation.

The use of cnd_signal() is safe only if the following conditions are met:

All threads must perform the same set of operations after waking up, which means that any thread can be selected to wake up and resume for a single invocation of cnd_signal().
Only one thread is required to wake upon receiving the signal.

The use of cnd_signal() can also be safe if each thread uses a unique condition variable.

The use of cnd_broadcast() avoids these problems because it wakes up all the threads associated with the condition variable, and because all the threads must reevaluate the predicate condition, one thread will find its test to be true, avoiding deadlock.

Noncompliant Code Example (`cnd_signal()`)

The following noncompliant code example consists of a given number of threads (5) that should execute one after another according to the step level assigned to each thread when it is created (serialized processing). The current_step variable holds the current step level and is incremented as soon as the respective thread finishes its processing. Finally, another thread is signaled so that the next step can be executed.

#include <stdio.h>
#include <stdlib.h>
#include <threads.h>

#define NTHREADS  5

mtx_t mutex;
cnd_t cond;


void *run_step(void *t) {
  static int current_step = 0;
  int my_step = (int)t;
  int result;

  if ((result = mtx_lock(&mutex)) != thrd_success) {
    /* Handle error condition */
  }

  printf("Thread %d has the lock\n", my_step);

  while (current_step != my_step) {
    printf("Thread %d is sleeping...\n", my_step);

    if ((result = cnd_wait(&cond, &mutex)) != thrd_success) {
      /* Handle error condition */
    }

    printf("Thread %d woke up\n", my_step);
  }

  /* Do processing... */
  printf("Thread %d is processing...\n", my_step);

  current_step++;

  /* Signal a waiting task */
  if ((result = cnd_signal(&cond)) != thrd_success) {
    /* Handle error condition */
  }

  printf("Thread %d is exiting...\n", my_step);

  if ((result = mtx_unlock(&mutex)) != thrd_success) {
    /* Handle error condition */
  }

  thrd_exit(NULL);
}


int main(int argc, char** argv) {
  int i;
  int result;
  thrd_t threads[NTHREADS];
  int step[NTHREADS];

  if ((result = mtx_init(&mutex, mtx_plain)) != thrd_success) {
    /* Handle error condition */
  }
  if ((result = cnd_init(&cond)) != thrd_success) {
    /* Handle error condition */
  }

  /* Create threads */
  for (i = 0; i < NTHREADS; i++) {
    step[i] = i;
    if ((result = thrd_create(&threads[i], run_step, (void *)step[i])) != thrd_success) {
      /* Handle error condition */
    }
  }

  /* Wait for all threads to complete */
  for (i = NTHREADS-1; i >= 0; i--) {
    if ((result = thrd_join(threads[i], NULL)) != thrd_success) {
      /* Handle error condition */
    }
  }

  if ((result = mtx_destroy(&mutex)) != thrd_success) {
    /* Handle error condition */
  }
  if ((result = cnd_destroy(&cond)) != thrd_success) {
    /* Handle error condition */
  }

  thrd_exit(NULL);
}

In this example, each thread has its own predicate because each requires current_step to have a different value before proceeding. Upon the signal operation (pthread_cond_signal()), any of the waiting threads can wake up. If, by chance, it is not the thread with the next step value, that thread will wait again (pthread_cond_wait()), resulting in a deadlock situation because no more signal operations will occur.

Consider the following example:

Time	Thread # (`my_step`)	`current_step`	Action
0	3	0	Thread 3 executes first time: predicate is `FALSE -> wait()`
1	2	0	Thread 2 executes first time: predicate is `FALSE -> wait()`
2	4	0	Thread 4 executes first time: predicate is `FALSE -> wait()`
3	0	0	Thread 0 executes first time: predicate is `TRUE -> current_step++; signal()`
4	1	1	Thread 1 executes first time: predicate is `TRUE -> current_step++; signal()`
5	3	2	Thread 3 wakes up (scheduler choice): predicate is `FALSE -> wait()`
6	—	—	Deadlock situation! No more threads to run, and a signal is needed to wake up the others.

This noncompliant code example violates the liveness property.

Compliant Solution (Using `cnd_broadcast()`)

This compliant solution uses the cnd_broadcast() method to signal all waiting threads instead of a single random one. Only the run_step() thread code from the noncompliant code example is modified, as follows:

void *run_step(void *t) {
  static int current_step = 0;
  int my_step = (int)t;
  int result;

  if ((result = mtx_lock(&mutex)) != thrd_success) {
    /* Handle error condition */
  }

  printf("Thread %d has the lock\n", my_step);

  while (current_step != my_step) {
    printf("Thread %d is sleeping...\n", my_step);

    if ((result = cnd_wait(&cond, &mutex)) != thrd_success) {
      /* Handle error condition */
    }

    printf("Thread %d woke up\n", my_step);
  }

  /* Do processing... */
  printf("Thread %d is processing...\n", my_step);

  current_step++;

  /* Signal ALL waiting tasks */
  if ((result = cnd_broadcast(&cond)) != thrd_success) {
    /* Handle error condition */
  }

  printf("Thread %d is exiting...\n", my_step);

  if ((result = mtx_unlock(&mutex)) != 0) {
    /* Handle error condition */
  }

  thrd_exit(NULL);
}

The fact that all threads will be awake solves the problem because each one ends up executing its predicate test; one will find its test to be true and will continue the execution until the end.

Compliant Solution (Using `cnd_signal()` but with a Unique Condition Variable per Thread)

Another way to solve the signal issue is to use a unique condition variable for each thread (maintaining a single mutex associated with it). In this case, the signal operation (cnd_signal()) wakes up the only thread waiting on it.

NOTE: The predicate of the signaled thread must be true; otherwise, a deadlock can occur.

#include <stdio.h>
#include <stdlib.h>
#include <threads.h>

#define NTHREADS  5
mtx_t mutex;
cnd_t cond[NTHREADS];


void *run_step(void *t) {
  static int current_step = 0;
  int my_step = (int)t;
  int result;

  if ((result = mtx_lock(&mutex)) != thrd_success) {
    /* Handle error condition */
  }

  printf("Thread %d has the lock\n", my_step);

  while (current_step != my_step) {
    printf("Thread %d is sleeping...\n", my_step);

    if ((result = cnd_wait(&cond[my_step], &mutex)) != thrd_success) {
      /* Handle error condition */
    }

    printf("Thread %d woke up\n", my_step);
  }

  /* Do processing... */
  printf("Thread %d is processing...\n", my_step);

  current_step++;

  /* Signal next step thread */
  if ((my_step + 1) < NTHREADS) {
    if ((result = cnd_signal(&cond[my_step+1])) != thrd_success) {
      /* Handle error condition */
    }
  }

  printf("Thread %d is exiting...\n", my_step);

  if ((result = mtx_unlock(&mutex)) != thrd_success) {
    /* Handle error condition */
  }

  thrd_exit(NULL);
}


int main(int argc, char** argv) {
  int i;
  int result;
  thrd_t threads[NTHREADS];
  int step[NTHREADS];

  if ((result = mtx_init(&mutex, mtx_plain)) != thrd_success) {
    /* Handle error condition */
  }

  for (i = 0; i< NTHREADS; i++) {
    if ((result = cnd_init(&cond[i])) != thrd_success) {
      /* Handle error condition */
    }
  }

  /* Create threads */
  for (i = 0; i < NTHREADS; i++) {
    step[i] = i;
    if ((result = thrd_create(&threads[i], run_step, (void *)step[i])) != thrd_success) {
      /* Handle error condition */
    }
  }

  /* Wait for all threads to complete */
  for (i = NTHREADS-1; i >= 0; i--) {
    if ((result = thrd_join(threads[i], NULL)) != thrd_success) {
      /* Handle error condition */
    }
  }

  if ((result = mtx_destroy(&mutex)) != thrd_success) {
    /* Handle error condition */
  }

  for (i = 0; i < NTHREADS; i++) {
    if ((result = cnd_destroy(&cond[i])) != thrd_success) {
      /* Handle error condition */
    }
  }

  thrd_exit(NULL);
}

In this compliant code, each thread has associated a unique condition variable that is signaled when that particular thread needs to be awakened. This solution turns out to be more efficient because only the desired thread is awakened.

Risk Assessment

Signaling a single thread instead of all waiting threads can pose a threat to the liveness property of the system.

Guideline	Severity	Likelihood	Remediation Cost	Priority	Level
CON38-C	low	unlikely	medium	P2	L3

Related Guidelines

CERT Oracle Secure Coding Standard for Java

THI04-J. Notify all waiting threads rather than a single thread

Bibliography

[Open Group]

pthread_cond_signal(), pthread_cond_broadcast()

Space shortcuts

Page tree

Noncompliant Code Example (`cnd_signal()`)

Compliant Solution (Using `cnd_broadcast()`)

Compliant Solution (Using `cnd_signal()` but with a Unique Condition Variable per Thread)

Risk Assessment

Related Guidelines

Bibliography

Space shortcuts

Page tree

CON38-C. Notify all threads waiting on a condition variable instead of a single thread

Noncompliant Code Example (cnd_signal())

Compliant Solution (Using cnd_broadcast())

Compliant Solution (Using cnd_signal() but with a Unique Condition Variable per Thread)

Risk Assessment

Related Guidelines

Bibliography

Noncompliant Code Example (`cnd_signal()`)

Compliant Solution (Using `cnd_broadcast()`)

Compliant Solution (Using `cnd_signal()` but with a Unique Condition Variable per Thread)