Search code examples
cunixforkpipewaitpid

Reading from a pipe to my child process


Hopefully a simple question. I'm trying to learn, simultaneously, fork(), pipe(), and waitpid() and running into some problems.

if (pipe(myout)<0 || pipe(myin)<0 || pipe(myerr)<0) { perror("Couldn't make pipes"); return; }
int childpid=fork();
if (childpid==0) { //child
    fdopen(myout[1], "w");
    fdopen(myin[1], "r");
    fdopen(myerr[1], "w");
    dup2(myout[1],  1);
    dup2(myin[1], 0);
    dup2(myerr[1], 2);
    printf("This should be seen\n");
    fclose(stdout); fclose(stdin); fclose(stderr);
    sleep(10);
    _exit(0);
 } else { //parent, wait on child
    printf("parent, monitoring\n");
    sim_out=fdopen(myout[0], "r");
    sim_in=fdopen(myin[0], "w");
    sim_err=fdopen(myerr[0], "r");
    printf("have my fds\n");
    int status;
    do {
        int ch;
        if (read(myout[0], &ch, 1)>0)
            write(1, &ch, 1);
        else printf("no go\n");
            waitpid(childpid, &status, WNOHANG);
    } while (!WIFEXITED(status) && !WIFSIGNALED(status));
}

I'm getting:

parent, monitoring have my fds T

before the program exits - that is, the loop runs only once. I have a check below that, and it's coming up WIFEXITED() so the process is supposed to have exited normally. But what bothers me is that there's a sleep(10) before that happens, and this happens immediately - not to mention that the child processes are left running for the remaining wait time.

I'm fundamentally misunderstanding something, clearly. My expectation was that the parent loop would block until it sees a character from the child, read it, then check to see if it was still alive. I certainly didn't expect waitpid() to set WIFEXITED when the child was still alive.

Where am I wrong?


Solution

  • I think I can see several issues. I'll try to mention them in order of appearance.

    • You should check fork for the return value -1 which indicates an error (no child will have been forked in this case).
    • All your calls to fdopen leak resources (depending on the implementation; the one on RHEL4 leaks). They return a FILE* which you could then use in fwrite etc. and then close by doing fclose on them. But you throw that value away. You don't need to open the pipe for reading/writing. Pipes are suitable for that when created.
    • The child should close the ends of the pipe it does not use. Add close (myin [1]); myin [1] = -1; close (myout [0]); myout [0] = -1; close (myerr [0]); myerr [0] = -1;
    • The dup2 are technically fine on all Linux variants I know, but it is customary to use the first fd of a pipe for reading and the other for writing. Thus, your dup2s would be best changed to dup2 (myin [0], STDIN_FILENO); dup2 (myout [1], STDOUT_FILENO); dup2 (myerr [1], STDERR_FILENO);
    • The parent should close the ends of the pipe it does not use. Add close (myin [0]); myin [0] = -1; close (myout [1]); myout [1] = -1; close (myerr [1]); myerr [1] = -1;
    • Your main problem: You check the possibly uninitialized status for your child's exit code. But waitpid has not yet been called. You should check waitpid's exit code and not evaluate status if it returns something other than childpid.

    Edit

    Since only the pipe ends that you actually need are open now, the operating system will detect a broken pipe for you. The pipe breaks when the child does fclose (stdout). The parent can still proceed to read all the data that may be in the pipe, but after that read will return zero, indicating a broken pipe.

    Thus, you can actually spare the call to waitpid. You could instead simply wait for read to return zero. This is not 100% equivalent, though, since your child closes its end of the pipe before it goes to sleep (causing the parent to proceed when all data have been read), whereas the waitpid version, of course, only proceeds when the child actually died.