Problems with piping for basic shell program

Question

I've compared this to previous posts involving piping and I can't seem to find the problem. Everything in the parent seems to be closed as it should. It works fine when I type in a valid command (ex "ls | grep a) but if it is not a valid command (ex "ls | grup a) the program stops responding to user input (it keeps running but it just doesn't do anything when you enter a command)

Main function:

int main() {
int i;
char **args; 

int pipeCheck = 0;
int argCount = -1;
int blank = 0;

while(1) {
    args = getln();

    if (args[0] != NULL){
        blank = 1;
        if (strcmp(args[0],"exit")==0) exit(0);
    }
    for(i = 0; args[i] != NULL; i++) {
    if (strcmp(args[i], "|")==0){
            pipeCheck = i;
        }
   }
   if (pipeCheck != 0){
            args[pipeCheck] = NULL;
            directPipe(args, pipeCheck, argCount, ampCheck);
        }
 }

} This is the function for piping in my program:

int directPipe(char ** args, int fileNumber, int argCount,int ampCheck){
    int fd[2];
    int child1,child2;
    int status;
    int i;
    char * piped[10000];
    int count = 0;

    for (i = (fileNumber+1); args[i] != NULL; i++){
        piped[count] = args[i];
        count++;
    }
    piped[count] = NULL;

    printf("\nPipe attempted...\n");

    pipe(fd);

    child1 = fork();
    if (child1==0){
        close(1);
        dup(fd[1]);
        close(fd[0]);
        close(fd[1]);

        execvp(args[0], args);
        printf("Unknown command, please try again.");
        exit(0);
    }

    child2 = fork();

    if (child2 ==0){

        close(0);
        close(fd[1]);
        dup(fd[0]);
        close(fd[0]);

        execvp(piped[0], piped);
        printf("Unknown command, please try again.");
        exit(0);
    }

    close(fd[1]);
    close(fd[0]);

    if (ampCheck == 0){
        while (wait(&status) != child1);
        while (wait(&status) != child2);
    }
    else{
        printf("\nampCheck = %d",ampCheck);
        sigset(child2, printer());
    }

    return (0);
}

Answer 1

Your problem is the pair of wait() loops:

while (wait(&status) != child1);
while (wait(&status) != child2);

In your scenario, the second child dies before the first does, so your collect its corpse in the first loop, but ignore it. Then the second loop goes into a busy wait because there are no children left any more.

At minimum, you need to do:

int corpse;

while ((corpse = wait(&status)) != -1 && corpse != child1 && corpse != child2)
    ;
while ((corpse = wait(&status)) != -1 && corpse != child1 && corpse != child2)
    ;

This handles children dying in either order — but only the two children. For a more general pipeline (three or more processes), you have to work harder — and use a single loop. The more general form will be something like:

int corpse;

while ((corpse = wait(&status)) != -1)
{
    if (record_death_of_child(corpse, status) == -1)
        break;
}

where your process creation code records the PIDs of the created processes, and the record_death_of_child() code deals with that list of PIDS and returns -1 when there are no more children to wait for in the current pipeline (and 0 otherwise). Or you can have it use some other heuristic to determine when to exit the loop. Note that if you have long running jobs in the background, any of them could die and that corpse would be collected in the loop. The 'record death' function would need to deal with such processes too — they can no longer be brought into the foreground, for example, and you need to report that they exited, etc.

You might end up using waitpid(), too, since you can arrange for that to not hang while there's a background process that's still running using WNOHANG.