Creating a Barrier for multi-thread-CodePudding

I am doing the Barrier task from mits081(link)

The thing to do is to implement the function barrier to stop threads from moving forward until all threads have reached. my implementation sometimes fail and sometimes succeed, so I print out some info inside the barrier function to help debug printf("%d :%d\n", bstate.nthread, syscall(__NR_gettid)); when succeed, the output will be like

when fail, it will stuck after printing the last line as shown.

Could someone kindly tell me what's wrong with my implementation? Thanks in advance

#include <stdlib.h>
#include <unistd.h>
#include <stdio.h>
#include <assert.h>
#include <pthread.h>
#include <sys/types.h>
#include <unistd.h>
#include <sys/syscall.h>
static int nthread = 1;
static int round = 0;

struct barrier
{
  pthread_mutex_t barrier_mutex;
  pthread_cond_t barrier_cond;
  int nthread; // Number of threads that have reached this round of the barrier
  int round;   // Barrier round
} bstate;

static void
barrier_init(void)
{
  assert(pthread_mutex_init(&bstate.barrier_mutex, NULL) == 0);
  assert(pthread_cond_init(&bstate.barrier_cond, NULL) == 0);
  bstate.nthread = 0;
}

static void
barrier()
{
  // YOUR CODE HERE
  //
  // Block until all threads have called barrier() and
  // then increment bstate.round.
  // //
  printf("%d :%d\n", bstate.nthread, syscall(__NR_gettid));

  if (  bstate.nthread % nthread == 0)
  {
    pthread_cond_broadcast(&bstate.barrier_cond); // wake up every thread sleeping on cond
    bstate.round  ;

  }
  else
    pthread_cond_wait(&bstate.barrier_cond, &bstate.barrier_mutex); // go to sleep on cond, releasing lock mutex, acquiring upon wake up
}

static void *
thread(void *xa)
{
  long n = (long)xa;
  long delay;
  int i;

  for (i = 0; i < 10; i  )
  {
    int t = bstate.round;
    assert(i == t);
    barrier();
    usleep(random() % 100);
  }
  // printf("a");

  return 0;
}

int main(int argc, char *argv[])
{
  pthread_t *tha;
  void *value;
  long i;
  double t1, t0;

  if (argc < 2)
  {
    fprintf(stderr, "%s: %s nthread\n", argv[0], argv[0]);
    exit(-1);
  }
  nthread = atoi(argv[1]);
  tha = malloc(sizeof(pthread_t) * nthread);
  srandom(0);

  barrier_init();

  for (i = 0; i < nthread; i  )
  {
    assert(pthread_create(&tha[i], NULL, thread, (void *)i) == 0);
      printf("%d\n", syscall(__NR_gettid));
  }
    

  for (i = 0; i < nthread; i  )
  {

    assert(pthread_join(tha[i], &value) == 0);
  }
  printf("OK; passed\n");
}

CodePudding user response：

You need all access to variables that you read/write from multiple threads simultaneously to be synchronized. You currently don't lock the mutex in barrier() which causes a race condition and undefined behavior. Your pthread_cond_wait may also wake up because of spurious wake-ups so you need to check the condition (that it's time to start the next round) every time it wakes up.

Example fix:

static void barrier() {
    // lock the mutex:
    pthread_mutex_lock(&bstate.barrier_mutex);

    printf("%d :%ld\n",   bstate.nthread, syscall(__NR_gettid));

    if(bstate.nthread == nthread) {
        // wake up every thread sleeping on cond
        printf("round %d done\n", bstate.round  );
        bstate.nthread = 0;
        pthread_cond_broadcast(&bstate.barrier_cond);
    } else {
        int lround = bstate.round;
        // sleep until the next round starts:
        do {
            pthread_cond_wait(&bstate.barrier_cond, &bstate.barrier_mutex);
        } while(lround == bstate.round);
    }

    // unlock the mutex:
    pthread_mutex_unlock(&bstate.barrier_mutex);
}

Also note that your thread function also reads from a variable (bstate.round) without synchronization which is also written to by other threads. Remove that:

static void* thread(void* xa) {
    long n = (long)xa;
    long delay;
    int i;

    for(i = 0; i < 10; i  ) {
        barrier();
        usleep(random() % 100);
    }

    return NULL;
}