Use while loop to make a thread wait till the lock variable is set to avoid race condition in C prgr-CodePudding

#include <stdio.h>
#include <pthread.h>

long mails = 0;
int lock = 0;

void *routine()
{
  printf("Thread Start\n");
  for (long i = 0; i < 100000; i  )
  {
    while (lock)
    {
    }

    lock = 1;
    mails  ;
    lock = 0;
  }
  printf("Thread End\n");
}

int main(int argc, int *argv[])
{
  pthread_t p1, p2;
  if (pthread_create(&p1, NULL, &routine, NULL) != 0)
  {
    return 1;
  }
  if (pthread_create(&p2, NULL, &routine, NULL) != 0)
  {
    return 2;
  }
  if (pthread_join(p1, NULL) != 0)
  {
    return 3;
  }
  if (pthread_join(p2, NULL) != 0)
  {
    return 4;
  }
  printf("Number of mails: %ld \n", mails);
  return 0;
}

In the above code each thread runs a for loop to increase the value of mails by 100000.
To avoid race condition is used lock variable along with while loop.
Using while loop in routine function does not help to avoid race condition and give correct output for mails variable.

CodePudding user response：

In C, the compiler can safely assume a (global) variable is not modified by other threads unless in few cases (eg. volatile variable, atomic accesses). This means the compiler can assume lock is not modified and while (lock) {} can be replaced with an infinite loop. In fact, this kind of loop cause an undefined behaviour since it does not have any visible effect. This means the compiler can remove it (or generate a wrong code). The compiler can also remove the lock = 1 statement since it is followed by lock = 0. The resulting code is bogus. Note that even if the compiler would generate a correct code, some processor (eg. AFAIK ARM and PowerPC) can reorder instructions resulting in a bogus behaviour.

To make sure accesses between multiple threads are correct, you need at least atomic accesses on lock. The atomic access should be combined with proper memory barriers for relaxed atomic accesses. The thing is while (lock) {} will result in a spin lock. Spin locks are known to be a pretty bad solution in many cases unless you really know what you are doing and all the consequence (in doubt, don't use them).

Generally, it is better to uses mutexes, semaphores and wait conditions in this case. Mutexes are generally implemented using an atomic boolean flag internally (with right memory barriers so you do not need to care about that). When the flag is mark as locked, an OS sleeping function is called. The sleeping function wake up when the lock has been released by another thread. This is possible since the thread releasing a lock can send a wake up signal. For more information about this, please read this. In old C, you can use pthread for that. Since C11, you can do that directly using this standard API. For pthread, it is here (do not forget the initialization).

CodePudding user response：

If you really want a spinlock, you need something like:

#include <stdatomic.h>

atomic_flag lock = ATOMIC_FLAG_INIT;

void *routine()
{
  printf("Thread Start\n");
  for (long i = 0; i < 100000; i  )
  {
    while (atomic_flag_test_and_set(&lock)) {}

    mails  ;
    atomic_flag_clear(&lock);
  }
  printf("Thread End\n");
}

However, since you are already using pthreads, you're better off using a pthread_mutex