Reusing a pipe for several messages

I'm writing a code where a parent forks several children, after creating a pipe for each one. In a while loop, the parent aks the user for some input, and communicates it to a child using a pipe, so the child needs to do some stuff about the input. The problem is that, after the child finishes, the parent process doesn't take control again, so somehow I'm stuck with the child, waiting for another input through the pipe. I'm really new into this things, so I don't know what I'm doing wrong

int main(int argc, char const *argv[]) {

  //Création de pipes anonymes (un pour chaque processus)
  int fd[4][2];
  for(int i = 0; i < 4; i++){
    pipe(fd[i]);
  }

  pid_t fils_pid[4];

  //Fork 4 processus fils
  for(int i = 0; i < 4; i++){
    if((fils_pid[i] = fork()) == 0){
 
      close(fd[i][W]); //Ferme les descripteurs d'écriture
      
      while(true){
        read(fd[i][R], query, 100);
        printf("Query: %s", query);
      }
      
      close(fd[i][R]);  
      exit(EXIT_SUCCESS);
    }
  }
 
  //children never reach here

  for(int i = 0; i < 4; i++){
    close(fd[i][R]);
  }

  while(true){
    int i = rand() % 4;
    printf("Enter a query: ");
    fgets(query, 100, stdin);

    write(fd[i][W], query, strlen(query) + 1);
    wait(NULL);
    }
  
  close(fd[t][W]);

  printf("Bye bye!\n");
  return 0;
}

Upvotes: 2

Views: 176

Answers (1)

Craig Estey
Craig Estey

Reputation: 33631

A few issues:

  1. The children just read forever. They never check for EOF.
  2. Parent wait in wrong place. Should be at end after closing all write ends of pipes.
  3. Child must close all write ends of pipes before starting its loop and not just its own.
  4. Child must close all read ends of pipes except its own.
  5. Otherwise, each child interferes with a given child seeing EOF on its pipe.
  6. Parent never detects EOF and never terminates its loop.
  7. Child can not rely on receiving an EOS (0x00) terminator in a single read because pipes can return short reads [or short writes].
  8. i.e. Child should fwrite to stdout instead of doing printf of received buffer.

Here is the refactored code. It is annotated with bugs and fixes:

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <fcntl.h>
#include <sys/wait.h>

enum {
    R = 0,
    W = 1
};

char query[100];

int
main(int argc, char const *argv[])
{

    // Création de pipes anonymes (un pour chaque processus)
    int fd[4][2];
    pid_t pid;

    for (int i = 0; i < 4; i++) {
        pipe(fd[i]);
        printf("PIPE %d R=%d W=%d\n",i,fd[i][R],fd[i][W]);
    }

    pid_t fils_pid[4];

    // Fork 4 processus fils
    for (int i = 0; i < 4; i++) {
        if ((fils_pid[i] = fork()) == 0) {
// NOTE/BUG: _each_ child must close _all_ write ends
// NOTE/BUG: _each_ child must close _all_ read ends _except_ its own
#if BUG
            close(fd[i][W]);            // Ferme les descripteurs d'écriture
#else
            for (int j = 0;  j < 4;  ++j) {
                close(fd[j][W]);
                if (j != i)
                    close(fd[j][R]);
            }
#endif

// NOTE/BUG: we can't guarantee an EOS terminator because of pipe short read
// NOTE/BUG: we never stop on EOF
#if BUG
            while (1) {
                read(fd[i][R], query, 100);
                printf("Query: %s", query);
            }
#else
            while (1) {
                ssize_t rlen = read(fd[i][R], query, sizeof(query));
                printf("Query/%d/%zd: ",i,rlen);
                if (rlen <= 0)
                    break;
                fwrite(query,1,rlen,stdout);
            }
#endif

            close(fd[i][R]);
            exit(EXIT_SUCCESS);
        }

#if 1
        printf("main: pid[%d]=%d\n",i,fils_pid[i]);
#endif
    }

    // children never reach here
    for (int i = 0; i < 4; i++) {
        printf("main: RCLOSE i=%d\n",i);
        close(fd[i][R]);
    }

    while (1) {
        int i = rand() % 4;

        printf("Enter a query: ");
        fflush(stdout);

// NOTE: bug we don't stop on EOF
#if BUG
        fgets(query, 100, stdin);
#else
        if (fgets(query, sizeof(query), stdin) == NULL) {
            printf("eof ...\n");
            break;
        }
#endif

// NOTE/FIX: stop sending to children if we get a blank line
#if ! BUG
        if (query[0] == '\n') {
            printf("stop ...\n");
            break;
        }
#endif

#if BUG
        write(fd[i][W], query, strlen(query) + 1);
#else
        write(fd[i][W], query, strlen(query));
#endif

#if BUG
        wait(NULL);
#endif
    }

// NOTE/BUG: we should close write ends of _all_ pipes
#if 0
    close(fd[t][W]);
#else
    for (int i = 0; i < 4; i++) {
        printf("main: WCLOSE i=%d\n",i);
        close(fd[i][W]);
    }
#endif

// NOTE/FIX: correct place to wait for children (_after_ closing pipes)
#if ! BUG
    while (1) {
        pid = wait(NULL);
        printf("wait: pid=%d\n",pid);
        if (pid < 0)
            break;
    }
#endif

    printf("Bye bye!\n");

    return 0;
}

In the above code, I've used cpp conditionals to denote old vs. new code:

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

#if 1
// new code
#endif

I've also used #if BUG and #if ! BUG for original code and bug fixes.

Note: this can be cleaned up by running the file through unifdef -k

Upvotes: 3

Related Questions