AlfaCoding
AlfaCoding

Reputation: 91

Message queue not receiving information as expected

I'm coding a program and I wanted to use a message queue in order to schedule the program ending. This is a schema of the code:

typedef struct {
    long id;
    int num;
}Message;

int disconnect = 0;

void disconnection() {
    disconnect = 1;
}

int main () {
   
    //...

    int n = fork();
    int id_queue = msgget(ftok("connection.h", 66), 0600|IPC_CREAT);

    switch(n) {

        case 0: //child process

            Message message;
            message.id = 1;
            message.num = 0;

            while (1) {
                // ...

                msgrcv(id_queue, (struct msgbuf *) &message, sizeof(int), 1, IPC_NOWAIT);

                if (message.num == 1) break;
            }

            write(1, "Exit successfull\n", strlen("exit successfulll")); //The child process does not reach this line

            msgctl(id_queue, IPC_RMID, (struct msqid_ds *)NULL);
        break;

        default: //main process
            signal(SIGINT, disconnection);

            while (!disconnect) { /* ... */ }

            Message msg;
            msg.id = 1;
            msg.num = 1;

            msgsnd(id_queue, (struct msgbuf *) &msg, sizeof(int), IPC_NOWAIT);

            wait(NULL); //waiting for child process to end
        break;

}

As you can see, the program works until I interrupt it with a SIGINT. The disconnection process in order to end the child process is to send a message that is just an int and if it's 1, child process loop should break. I have tested it and it doesn't enter (doesn't print the write after the while(1) loop) so I'm figuring out I have some errors sending or receiving the data through the message queue.

What's wrong?

Also, the program ends after I send SIGINT interruption and wait(NULL) is not waiting for child process to end, as the write doesn't print through the screen...

Upvotes: 1

Views: 997

Answers (1)

Craig Estey
Craig Estey

Reputation: 33631

There were a number of issues.

When you do ctrl-c, the signal will be sent both parent and the child. The child must disable the signal or it will be killed by the default signal action.

The parent should not use IPC_NOWAIT when doing the msgsnd.

In the child, it can do IPC_NOWAIT, but we have to check the return value and loop if it returns -1 (i.e. the message contents is not valid otherwise).

You can not put a scoped declaration inside a case [IIRC]. Better to move these to function scope.

disconnect should be declared volatile. This is sufficient for your use case. In the more general case [as others have mentioned], you should use stdatomic.h primitives.

There's a lot of hardcoding of special constants (e.g. 1 for .num).

The length arguments to msgsnd and msgrcv are hardwired with sizeof(int). This won't scale too well [if other fields are added to the Message struct].

You can use ftok to get a key_t value. But, here, it's overkill. Just use IPC_PRIVATE in the parent [before the fork]. That's what most programs do if they're only doing fork


I've refactored the code. I've marked changes using cpp conditionals:

#if 0
// old code
#else
// new code
#endif

Anyway, here is the working code:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>
#include <signal.h>
#include <sys/wait.h>
#include <sys/ipc.h>
#include <sys/msg.h>

#define fault(_fmt...) \
    do { \
        printf(_fmt); \
        exit(1); \
    } while (0)

typedef struct {
    long id;
    int num;
} Message;

// equates to id
enum {
    MSGID_ANY,                      // receive any message
    MSGID_TOCLD,                    // send/receive messages to child
    MSGID_TOPAR,                    // send/receive messages to parent
};

// equates to num
enum {
    NUM_DO_X,                       // perform "x" ...
    NUM_DO_Y,                       // perform "y" ...
    NUM_STOP,                       // stop request
};

#if 0
int disconnect = 0;
#else
volatile int disconnect = 0;
#endif

void
disconnection()
{
    disconnect = 1;
}

int
main()
{
#if 1
    Message msg;
    int err;
#endif

    // ...

#if 0
    int n = fork();
    int id_queue = msgget(ftok("connection.h", 66), 0600 | IPC_CREAT);
#else
    int id_queue = msgget(IPC_PRIVATE, 0600 | IPC_CREAT);
    printf("parent: id_queue=%d\n",id_queue);
    int n = fork();
#endif

    switch (n) {
    case 0:  // child process
#if 0
        Message message;
#endif
        msg.id = MSGID_TOCLD;
        msg.num = 0;

#if 1
        signal(SIGINT, SIG_IGN);
#endif

        while (1) {
            // ...
#if 0
            msgrcv(id_queue, (struct msgbuf *) &msg, sizeof(int), 1,
                IPC_NOWAIT);
#else
            err = msgrcv(id_queue, (struct msgbuf *) &msg,
                sizeof(msg) - sizeof(msg.id), MSGID_TOCLD, IPC_NOWAIT);
            if (err < 0)
                continue;
            printf("child: got num=%d\n",msg.num);
#endif
            if (msg.num == NUM_STOP)
                break;
        }

        // The child process does not reach this line
#if 0
        write(1, "Exit successfull\n", strlen("exit successfulll"));
#else
        printf("Exit successful\n");
#endif

        msgctl(id_queue, IPC_RMID, (struct msqid_ds *) NULL);
        break;

    default:  // main process
        signal(SIGINT, disconnection);

        while (!disconnect) {
        }
        printf("Parent got signal\n");

#if 0
        Message msg;
#endif
        msg.id = MSGID_TOCLD;
        msg.num = NUM_STOP;

#if 0
        msgsnd(id_queue, (struct msgbuf *) &msg, sizeof(int), IPC_NOWAIT);
#else
        err = msgsnd(id_queue, (struct msgbuf *) &msg,
            sizeof(msg) - sizeof(msg.id), 0);
        if (err < 0)
            fault("parent: msgsnd fail -- %s\n",strerror(errno));
#endif

        // waiting for child process to end
        wait(NULL);
        break;
    }

    return 0;
}

Upvotes: 1

Related Questions