3

I'm trying to implement interprocess communication by using POSIX signals in C, especially I'm writing Ping-Pong problem. So here's my source code:

#define CHILD 0
#define PARENT 1

int flag[2];

void handler(int sig) {
    if (sig == SIGUSR1) {
        flag[PARENT] = 1;
    } else {
        flag[CHILD] = 1;
    }   
    return;
}

void child_process() {
    while (1) {
        printf("Ping!\n");
        kill(getppid(), SIGUSR2);
        while (flag[PARENT] == 0) { }
    }
    return;
}

void parent_process(pid_t t) {
    while (1) {
        //kill(t, SIGUSR1);
        while (flag[CHILD] == 0) { }
        printf("Pong!\n");

        kill(t, SIGUSR1);
    }
    return;
}

void setup() {
    flag[CHILD] = 0;
    flag[PARENT] = 0;
    signal(SIGUSR1, handler);
    signal(SIGUSR2, handler);
}

int main(int argc, char* argv[]) {  
    setup();
    pid_t t = fork();
    if (t == 0) {
        child_process();
    } else {
        parent_process(t);
    }
    return 0;
}

My program is not working properly, because sometimes I get "Pong!" "Pong!" "Pong!" or "Ping!" "Ping!" output. What's the problem?

And one more question, is my way of handling signals correct? Or there are more advanced ways to do it?

inaumov17
  • 1,329
  • 11
  • 18
  • You should be using [`sigaction()`](http://pubs.opengroup.org/onlinepubs/9699919799/functions/sigaction.html) rather than [`signal()`](http://pubs.opengroup.org/onlinepubs/9699919799/functions/signal.html). See [What is the difference between `sigaction()` and `signal()`?](https://stackoverflow.com/questions/231912/what-is-the-difference-between-sigaction-and-signal/232711#232711) for reasons why. – Jonathan Leffler Apr 13 '14 at 20:33

3 Answers3

2

(1) Parent and child do not share the same memory. flag[CHILD] and flag[PARENT] will never know about each other because they are different copies in different processes.

(2) Yes, pretty much everything about your signal handling is wrong for what you are trying to do. You are trying to synchronize the signals so you need to use a mechanism that actually synchronizes them e.g. sigsuspend.

#define _POSIX_C_SOURCE 1
#include <stdio.h>
#include <unistd.h>
#include <signal.h>
#include <stdlib.h>
#include <sys/types.h>
#include <errno.h>

#define errExit(msg) do { perror(msg); exit(EXIT_FAILURE); } while (0)

void sig_hand(int sig) {}

sigset_t saveMask, blockMask;

void child_process()
{
    int x = 0;

    while(x < 10)
    {
        if (sigsuspend(&saveMask) == -1 && errno != EINTR)
            errExit("sigsuspend");

        printf("Pong %d!\n", ++x);

        kill(getppid(), SIGUSR1);
    }

    return ;
}

void parent_process(pid_t pid)
{
    int y = 0;

    while (y < 10)
    {
        printf("Ping %d!\n", ++y);

        kill(pid, SIGUSR1);

        if (sigsuspend(&saveMask) == -1 && errno != EINTR)
            errExit("sigsuspend");
    }

    return ;
}


int main(int argc, char* argv[])
{
    //block SIGUSR1 in parent & child until ready to process it
    sigemptyset(&blockMask);
    sigaddset(&blockMask, SIGUSR1);

    if (sigprocmask(SIG_BLOCK, &blockMask, &saveMask) == -1)
        errExit("sigprocmask");

    //set up signal handler for parent & child
    struct sigaction sa;
    sigemptyset(&sa.sa_mask);
    sa.sa_flags = 0;
    sa.sa_handler = sig_hand;

    if (sigaction(SIGUSR1, &sa, NULL) == -1)
        errExit("sigaction");

    pid_t pid = fork();

    if (pid == 0)
        child_process();
    else
        parent_process(pid);

    return 0;
}
Duck
  • 26,924
  • 5
  • 64
  • 92
0

Although it may not be your problem, remember anytime you are modifying variables asynchronous to program flow, you need to make those variables volatile so that the compilers does not optimize the accesses to them away.

I would think that semaphore.h has much more useful tools (sem_open, sem_post, sem_wait, sem_trywait).

dennis
  • 221
  • 1
  • 5
  • Welcome to Stack Overflow. What you say about `volatile` is true, but not all that relevant since two separate processes here do not share variables (each has its own copy of the variables). I suspect [`pause()`](http://pubs.opengroup.org/onlinepubs/9699919799/functions/pause.html) would be a better tool to use in place of the busy waits. – Jonathan Leffler Apr 13 '14 at 20:37
0

I'd use the sigaction() and pause() functions, along with nanosleep() to rate-limit the activity.

#include <errno.h>
#include <signal.h>
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <unistd.h>

enum { MAX_PINGS = 10 };

static sig_atomic_t sig_num;

static void err_exit(const char *fmt, ...)
{
    int errnum = errno;
    va_list args;
    va_start(args, fmt);
    vfprintf(stderr, fmt, args);
    va_end(args);
    if (errnum != 0)
        fprintf(stderr, ": (%d) %s", errnum, strerror(errnum));
    putc('\n', stderr);
}

static void catcher(int sig)
{
    sig_num = sig;
}

static void child_process(void)
{
    struct timespec nap = { .tv_sec = 0, .tv_nsec = 100000000 };

    while (1)
    {
        pause();
        printf("Pong!\n");
        nanosleep(&nap, 0);
        kill(getppid(), SIGUSR1);
    }
}

static void parent_process(pid_t pid)
{
    struct timespec nap = { .tv_sec = 0, .tv_nsec = 100000000 };

    for (int pings = 0; pings < MAX_PINGS; pings++)
    {
        printf("Ping %d!\n", pings);
        nanosleep(&nap, 0);
        kill(pid, SIGUSR1);
        pause();
    }
    kill(pid, SIGTERM);
}

int main(void)
{
    struct sigaction sa;
    sigemptyset(&sa.sa_mask);
    sa.sa_flags = 0;
    sa.sa_handler = catcher;

    if (sigaction(SIGUSR1, &sa, NULL) == -1)
        err_exit("Failed to set SIGUSR1 handler");

    pid_t pid = fork();

    if (pid < 0)
        err_exit("Failed to fork()");
    else if (pid == 0)
        child_process();
    else
        parent_process(pid);

    return 0;
}

The variable sig_num is there to quell complaints from the compiler about unused arguments (to the catcher function). The signal catcher is set before the fork(). The child process pauses until a signal arrives; then prints 'Pong!', takes a nap for 1/10 seconds, and then signals the parent process to wake. The parent process prints 'Ping!', takes a nap, signals the child process, and pauses until a signal arrives. It limits the loops to 10 (enough to show it is working), and when it is done, terminates the child before exiting.

Example output

$ ./pingpong
Ping 0!
Pong!
Ping 1!
Pong!
Ping 2!
Pong!
Ping 3!
Pong!
Ping 4!
Pong!
Ping 5!
Pong!
Ping 6!
Pong!
Ping 7!
Pong!
Ping 8!
Pong!
Ping 9!
Pong!
$

Clearly, it would not be hard to print a counter on the 'Pong' values too.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278