Why does exec print its output after the program exits?

Question

I'm writing a program to pipe one command to another. Inputs will be from the command line:

$ ./a.out ls '|' wc
c2 PID 6804
c1 PID 6803
PARENT PID 6802
$       2       2      17

Why does the output print after the prompt returns. Is there any way to prevent that?

This is the code I've written:

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

int main(int argc, char * argv[])
{
    if(argc <= 1 )
    {
        printf("ERROR: No arguments passed\n");
        printf("USAGE: ./pipe <command 1> | <command 2>\n");
        return 1;
    }

    char * cmd1[50];
    char * cmd2[50];
    int cmd1_arg = 0;
    int cmd2_arg = 0;
    int pipe_num = 0;
    
    for(int cla = 1; cla<argc; cla++)
    {
        if( !strcmp(argv[cla],"|") )        
            pipe_num++;
        else if(pipe_num == 0)        
            cmd1[cmd1_arg++] = argv[cla];
        else if(pipe_num == 1)
            cmd2[cmd2_arg++] = argv[cla];
    }

    cmd1[cmd1_arg] = (char *)NULL;
    cmd2[cmd2_arg] = (char *)NULL;    

    if(pipe_num != 1)
    {
        printf("ERROR: Insufficient arguments passed\n");
        printf("USAGE: ./pipe <command 1> | <command 2>\n");
        return 1;
    }

    int pipe_fd[2];
    pipe(pipe_fd);   

    pid_t pid = fork();
    if(pid == -1)
    {
        perror("FORK FAILED");
        return 1;
    }    

    if(pid != 0)
    {          
        pid_t cmd_pid = fork();
        if(cmd_pid == -1)
        {
            perror("FORK FAILED");
            return 1;
        }

        if(cmd_pid != 0)
        {   
            waitpid(pid,NULL,0);                     
            waitpid(cmd_pid,NULL,WNOHANG);            
            printf("PARENT PID %d\n",getpid());            
        }

        if(cmd_pid == 0)
        {
            printf("c2 PID %d\n",getpid());
            close(pipe_fd[1]);
            int stdin_fd = dup(0);
            close(0);
            dup(pipe_fd[0]);            
            if(execvp(cmd2[0],cmd2) == -1 ) perror("CMD2 FAIL"); 
            close(0);
            dup(stdin_fd);
        }        
    }

    if(pid == 0)
    {
        printf("c1 PID %d\n",getpid());        
        close(pipe_fd[0]);        
        int stdout_fd = dup(1);        
        close(1);                
        int test = dup(pipe_fd[1]);
        if( execvp(cmd1[0],cmd1) == -1 ) perror("CMD1 FAIL");
        close(1);        
        dup(stdout_fd);
    }

    return 0;
}

Answer 1

You have:

    waitpid(cmd_pid,NULL,WNOHANG);

By including the WNOHANG option, you're telling waitpid() to not wait for the process to terminate if it hasn't already terminated.

My guess is that you added that because your program hangs if you don't include it. That's because the original parent still has an open file descriptor to the write-end of the pipe, so the reading process is still blocked waiting for input on that pipe.

Here's a revised version of your program that closes the pipe file descriptors, and that does not use WNOHANG.

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

int main(int argc, char *argv[])
{
    if (argc <= 1) {
        printf("ERROR: No arguments passed\n");
        printf("USAGE: ./pipe <command 1> | <command 2>\n");
        return 1;
    }

    char *cmd1[50];
    char *cmd2[50];
    int cmd1_arg = 0;
    int cmd2_arg = 0;
    int pipe_num = 0;

    for (int cla = 1; cla < argc; cla++) {
        if (!strcmp(argv[cla], "|")) {
            pipe_num++;
        } else if (pipe_num == 0) {
            cmd1[cmd1_arg++] = argv[cla];
        } else if (pipe_num == 1) {
            cmd2[cmd2_arg++] = argv[cla];
        }
    }

    cmd1[cmd1_arg] = NULL;
    cmd2[cmd2_arg] = NULL;

    if (pipe_num != 1) {
        printf("ERROR: Insufficient arguments passed\n");
        printf("USAGE: ./pipe <command 1> | <command 2>\n");
        return 1;
    }

    int pipe_fd[2];

    if (pipe(pipe_fd) < 0) {
        perror("pipe");
        return 1;
    }

    const pid_t pid = fork();

    if (pid < 0) {
        perror("fork");
        return 1;
    } else if (pid != 0) {
        const pid_t cmd_pid = fork();

        if (cmd_pid < 0) {
            perror("fork");
            return 1;
        } else if (cmd_pid != 0) {
            printf("PARENT PID %d\n", getpid());

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

            if (waitpid(pid, NULL, 0) < 0) {
                perror("waitpid");
            }

            if (waitpid(cmd_pid, NULL, 0) < 0) {
                perror("waitpid");
            }
        } else {
            printf("c2 PID %d\n", getpid());

            if (dup2(pipe_fd[0], STDIN_FILENO) < 0) {
                perror("dup2");
                return 1;
            }
            close(pipe_fd[0]);
            close(pipe_fd[1]);

            if (execvp(cmd2[0], cmd2) < 0) {
                perror("CMD2 FAIL");
                return 1;
            }
        }
    } else {
        printf("c1 PID %d\n", getpid());

        if (dup2(pipe_fd[1], STDOUT_FILENO) < 0) {
            perror("dup2");
            return 1;
        }
        close(pipe_fd[0]);
        close(pipe_fd[1]);

        if (execvp(cmd1[0], cmd1) < 0) {
            perror("CMD1 FAIL");
            return 1;
        }
    }

    return 0;
}

A run of this program gives me:

 ./a.out echo 1 2 3 '|' wc
PARENT PID 20412
c1 PID 20413
c2 PID 20414
      1       3       6
$

Answer 2

Your original process, the parent to the child processes, runs this:

waitpid(pid,NULL,0);                     
waitpid(cmd_pid,NULL,WNOHANG);            
printf("PARENT PID %d\n",getpid());  

That is, it waits for the first child to exit, makes a system call to check if the second one exited, but because of WNOHANG doesn't actually wait for it. The check is moot anyway, since the program doesn't use the return value from waitpid(). Then the parent prints its PID, and continues along the function, through the other two if-statements, and exits via return 0.

At that point, the second child process may still be running, or it may not be. Without explicit syncronization, there's no guarantees about which order things happen. Also, the shell has no knowledge of the child processes your program launches and has no mechanism to wait for them. The main process has to wait*() for them.

Before you continue with the code, get rid of the mess of nested if-statements, and invest in some clearer structure. A usual method to simplify code like that is to have the child processes immediately call into a function and _exit() there or immediately after the function returns. Right now, if your exec*() calls fail, the children also continue down executing the same code instead of exiting.