Process Synchronisation in C does not execute the first time

Question

So I am having this issue with a Process Synchronization program in C.

I am supposed to make a code that, using fork(), will produce something like this:

PARENT
PARENT
CHILD
PARENT
CHILD
PARENT

Using a code I found here, I was able to make it work but for some reasons, the first result that comes to screen is messed while all the others work just fine.

To compile, type : gcc test.c display.c -o test -pthread

Anyway, here is the code I am testing (I repeat: it's not my code):

#include <semaphore.h>
#include <stdio.h>
#include <errno.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/mman.h>

int main(void)
{
  int i;
  /* place semaphore in shared memory */
  sem_t *sema = mmap(NULL, sizeof(sema), PROT_READ |PROT_WRITE,MAP_SHARED|MAP_ANONYMOUS,-1, 0);
  /* create/initialize semaphore */
  if ( sem_init(sema, 1, 0) < 0)
  {
    perror("sem_init");
    exit(EXIT_FAILURE);
  }
  int nloop=10;
  int pid = fork();
  if (pid == 0)
  { 
    for (i = 0; i < nloop; i++)
    {
      // child unlocks semaphore
      display("CHILD\n");
      if (sem_post(sema) < 0)
          perror("sem_post");
      sleep(1);
    }
    if (munmap(sema, sizeof(sema)) < 0)
    {
      perror("munmap");
      exit(EXIT_FAILURE);
    }
      exit(EXIT_SUCCESS);
  }
  else
  {
    for (i = 0; i < nloop; i++)
    { // parent starts waiting
      display("PARENT\n");
      if (sem_wait(sema) < 0) 
        perror("sem_wait");
    // parent finished waiting
    }
    if (sem_destroy(sema) < 0)
    {
      perror("sem_destroy failed");
      exit(EXIT_FAILURE);
    }
    if (munmap(sema, sizeof(sema)) < 0)
    {
      perror("munmap failed");
      exit(EXIT_FAILURE);
    }
    exit(EXIT_SUCCESS);
  }
}

Here is the output:

PACREHNT
ILD
PARENT
CHILD
PARENT
CHILD
PARENT
CHILD
PARENT
CHILD
PARENT
CHILD
PARENT
CHILD
PARENT
CHILD
PARENT
CHILD
PARENT
CHILD

Why does this happen in the beginning?

Answer 1

You can't write a program that alternates between a parent and a child process with a single semaphore (without resorting to some form of busy waiting with flags or something), because both processes will race to acquire the semaphore; there is no way to predict which process will acquire it first. Your code (other than the first iteration) seems to work as expected because the child sleeps for a very long time, but technically it is still a race condition, there is no guarantee that the parent will get a chance to acquire the semaphore before the child wakes up (although it is very unlikely).

So, you need 2 semaphores: one used by the child to notify the parent that it's his turn, and another used by the parent to notify the child. To pick who starts first, initialize the corresponding semaphore to 1 (and the other to 0).

Also, this is wrong:

sem_t *sema = mmap(NULL, sizeof(sema), PROT_READ |PROT_WRITE,MAP_SHARED|MAP_ANONYMOUS,-1, 0);

The 2nd argument should be sizeof(*sema) since you want to allocate memory for the semaphore object, not for the pointer.

You never #include "display.h", you probably should.

Error handling could be improved, but for this toy program I don't think it's a big deal.

Here's a working version using the 2-semaphore approach (this initializes the parent semaphore to 1, so the parent will start first):

#include <semaphore.h>
#include <stdio.h>
#include <errno.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/mman.h>

int main(void)
{
    int i;
    /* place semaphore in shared memory */
    sem_t *child_sem = mmap(NULL, sizeof(*child_sem), PROT_READ |PROT_WRITE,MAP_SHARED|MAP_ANONYMOUS,-1, 0);
    sem_t *parent_sem = mmap(NULL, sizeof(*parent_sem), PROT_READ |PROT_WRITE,MAP_SHARED|MAP_ANONYMOUS,-1, 0);

    /* create/initialize semaphore */
    if ( sem_init(child_sem, 1, 0) < 0)
    {
        perror("sem_init");
        exit(EXIT_FAILURE);
    }

    if (sem_init(parent_sem, 1, 1) < 0) {
        perror("sem_init");
        exit(EXIT_FAILURE);
    }

    int nloop=10;
    int pid = fork();
    if (pid == 0)
    {
        for (i = 0; i < nloop; i++)
        {
            if (sem_wait(child_sem) < 0)
                perror("sem_wait");
            display("CHILD\n");
            if (sem_post(parent_sem) < 0)
                perror("sem_post");
            sleep(1);
        }
    }
    else
    {
        for (i = 0; i < nloop; i++)
        { // parent starts waiting
            if (sem_wait(parent_sem) < 0) 
                perror("sem_wait");
            display("PARENT\n");
            if (sem_post(child_sem) < 0)
                perror("sem_post");
        }
    }
}

I removed the munmap(2) and sem_destroy(3) calls for conciseness, and because they are unnecessary since the process is exiting anyway.

Notice that both processes follow the same pattern: wait for their semaphore, do work, notify the other process' semaphore. This is a nice opportunity to do some refactoring and move it all to a function that receives the string to display, the semaphore to wait on, and the semaphore to notify afterwards.

You should also get used to compiling with -Wall.