Killing forked child process kills parent in SIGCHLD handler

Question

I'm writing a simple shell in C that supports executing normal Linux commands as well as a couple of built in commands I wrote myself (like "quit" for exiting the shell). The shell forks a new child for each command entered. I have a SIGCHLD handler that reaps zombie children when a child process finishes. My problem is whenever I run a background process, the parent (the actual shell program) is also killed when the SIGCHLD handler is called.

I've already looked at this post and made a little SIGPIPE handler, but it never gets called, so I don't think that's the issue.

This eval function processes each command typed in and decides whether to run it in the foreground or background:

void eval(char *cmdline) 
{
    char *argv[MAXARGS];
    char buf[MAXLINE];
    int bg;
    pid_t pid;
    sigset_t mask_all, mask_one, prev_one;

    sigfillset(&mask_all);
    sigemptyset(&mask_one);
    sigaddset(&mask_one, SIGCHLD);


    strcpy(buf, cmdline);
    /* Is it a background command (ends in &) ? */
    bg = parseline(buf, argv);

    /*Ignore empty lines */
    if(argv[0] == NULL)
        return;

    /*builtin_cmd just handles my own custom commands */
    if(!builtin_cmd(argv)) {

        /*Block SIGCHLD signals */
        sigprocmask(SIG_BLOCK, &mask_one, &prev_one);
        if((pid = fork()) == 0) {
            /* Unblock SIGCHLD */
            sigprocmask(SIG_SETMASK, &prev_one, NULL);
            if(execve(argv[0], argv, environ) < 0) {
                fprintf(stderr, "%s: Command not found\n", argv[0]);
                fflush(stderr);
                exit(0);
            }
        }

        /* Block all signals */
        sigprocmask(SIG_BLOCK, &mask_all, NULL);
        setpgid(pid, 0);
        if(!bg) {
            /*Wait for foreground job to complete */
            int status;
            if(waitpid(pid, &status, 0) < 0) {
                fprintf(stderr, "wait pid error");
            }
        } else {
            /* Add the job to the jobs array */
            /* BG just means it's in the background */
            addjob(jobs, pid, BG, cmdline);
            int jid = (*getjobpid(jobs, pid)).jid;
            printf("[%d] (%d) %s", jid, pid, cmdline);
        }
        /*Unblock signals */
        sigprocmask(SIG_SETMASK, &prev_one, NULL);
    }

    return;
}

Here's my SIGCHLD handler:

void sigchld_handler(int sig) 
{
    int olderrno = errno;
    sigset_t mask_all, prev_all;
    sigfillset(&mask_all);

    /*Reap zombie */
    pid_t pid;
    if((pid = waitpid(-1, NULL, WNOHANG)) > 0) {
        sigprocmask(SIG_BLOCK, &mask_all, &prev_all);   //Block all signals
        deletejob(jobs, pid);                           //Delete job from jobs array
        sigprocmask(SIG_SETMASK, &prev_all, NULL);      //Unblock signals
    }

    //WHY DOES THIS EXECUTE FOR BACKGROUND JOBS???
    if(errno != ECHILD) {
        printf("waitpid error");
    }
    errno = olderrno;

    return;
}

The waitpid kills the children, which I know from print statements I've put inside of it. But when I run background processes, the parent is also killed and the "waitpid error" message is printed.

brett@brett-linux:~/Desktop/Shell$ ./tsh
tsh> /bin/sleep 1 &
[1] (7223) /bin/sleep 1 &
tsh> waitpid error
brett@brett-linux:~/Desktop/Shell$

As you can see, the program just dies. Why is this happening?

By the way, fflush(stderr) is not needed, as stderr should not be buffered. — Some programmer dude
Lastly, never check errno unless a call that sets it actually fails. Otherwise the value of errno is indeterminate. And note that printf is not signal-safe. — Some programmer dude
@Someprogrammerdude Wow, you're right - removing the printf fixes the problem. My textbook talks a bit about signal safe functions, I'll have to read over that again. — Brett Fisher

Brett Fisher Brett Fisher · Accepted Answer · 2019-01-20T18:29:06

0

votes

Removing the non-signal safe function printf fixes the problem.

Killing forked child process kills parent in SIGCHLD handler

1 Answers