0
#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.
Gopendra
  • 15
  • 1
  • 7
  • 3
    `int lock` is not a lock of any sort. Just calling it a lock doesn't eliminate race conditions. You have to use **real** locking objects like mutexes or semaphores. – Andrew Henle Oct 09 '22 at 16:53

3 Answers3

3

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).

Jérôme Richard
  • 41,678
  • 6
  • 29
  • 59
1

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

Chris Dodd
  • 119,907
  • 13
  • 134
  • 226
1

Jérôme Richard told you about ways in which the compiler could optimize the sense out of your code, but even if you turned all the optimizations off, you still would be left with a race condition. You wrote

while (lock) { }
lock=1;
...critical section...
lock=0;

The problem with that is, suppose lock==0. Two threads racing toward that critical section at the same time could both test lock, and they could both find that lock==0. Then they both would set lock=1, and they both would enter the critical section...

...at the same time.

In order to implement a spin lock,* you need some way for one thread to prevent other threads from accessing the lock variable in between when the first thread tests it, and when the first thread sets it. You need an atomic (i.e., indivisible) "test and set" operation.

Most computer architectures have some kind of specialized op-code that does what you want. It has names like "test and set," "compare and exchange," "load-linked and store-conditional," etc. Chris Dodd's answer shows you how to use a standard C library function that does the right thing on whatever CPU you happen to be using...

...But don't forget what Jérôme said.*

* Jérôme told you that spin locks are a bad idea.

Solomon Slow
  • 25,130
  • 5
  • 37
  • 57