Pipe guarantee to close after the child has exited

Question

In the code below, is it safe to rely on read() failure to detect termination of child?

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

int main(void)
{
   int pipefd[2];
   pipefd[0] = 0;
   pipefd[1] = 0;
   pipe(pipefd);

   pid_t pid = fork();

   if (pid == 0)
   {
      // child
      close(pipefd[0]);    // close unused read end
      while ((dup2(pipefd[1], STDOUT_FILENO) == -1) && (errno == EINTR)) {}             // send stdout to the pipe
      while ((dup2(pipefd[1], STDERR_FILENO) == -1) && (errno == EINTR)) {} // send stderr to the pipe
      close(pipefd[1]);    // close unused write end

      char *argv[3];
      argv[0] = "worker-app";
      argv[1] = NULL;
      argv[2] = NULL;
      execvp("./worker-app", argv);
      printf("failed to execvp, errno %d\n", errno);
      exit(EXIT_FAILURE);
   }
   else if (pid == -1)
   {
   }
   else
   {
      // parent
      close(pipefd[1]);  // close the write end of the pipe in the parent

      char buffer[1024];
      memset(buffer, 0, sizeof(buffer));
      while (1) // <= here is it safe to rely on read below to break from this loop ?
      {
        ssize_t count = read(pipefd[0], buffer, sizeof(buffer)-1);
        printf("pipe read return %d\n", (int)count);
        if (count > 0)
        {
          printf("child: %s\n", buffer);
        }
        else if (count == 0)
        {
          printf("end read child pipe\n", buffer);
          break;
        }
        else if (count == -1)
        {
          if (errno == EINTR)
          {   continue;
          }
          printf("error read child pipe\n", buffer);
          break;
        }
      }

      close(pipefd[0]); // close read end, prevent descriptor leak

      int waitStatus = 0;
      waitpid(pid, &waitStatus, 0);
  }

  fprintf(stdout, "All work completed :-)\n");
  return EXIT_SUCCESS;
}

Should I add something in the while(1) loop to detect child termination? What specific scenario could happen and break this app ?

Some thoughts of improvements below. However would I just waste CPU cycles?

1. Use kill with special argument 0 that won't kill the process but just check if it is responsive: if (kill(pid, 0)) { break; /* child exited */ }; /* If sig is 0, then no signal is sent, but error checking is still performed; this can be used to check for the existence of a process ID or process group ID. https://linux.die.net/man/2/kill */

2. Use waitpid non-blocking in the while(1) loop to check if child has exited.

3. Use select() to check for pipe readability to prevent read() from possibly hanging?

Thanks!

Answer 1

Regarding your ideas:

• If the child spawns children of its own, the read() won't return 0 until all of its descendants either die or close stdout and stderr. If it doesn't, or if the child always outlives all of its descendants, then just waiting for read() to return 0 is good enough and won't ever cause a problem.
• If the child dies but the parent hasn't yet wait(2)ed on it, then kill(pid, 0) will succeed as if the child were still alive (at least on Linux), so this isn't an effective check from within your parent program.
• A non-blocking waitpid() on its own would appear to fix the problem with the child having children of its own, but would actually introduce a subtle race condition. If the child exited right after the waitpid() but before the read(), then the read() would block until the rest of the descendants exited.
• On its own, if you used select() in a blocking way, it's no better than just calling read(). If you used select() in a non-blocking way, you'd just end up burning CPU time in a loop.

What I'd do:

• Add a no-op signal handler function for SIGCHLD, just so that it causes EINTR when it occurs.
• Block SIGCHLD in the parent before you start looping.
• Use non-blocking reads, and use pselect(2) to block to avoid spinning the CPU forever.
• During the pselect, pass in a sigset_t that doesn't have SIGCHLD blocked, so that it's guaranteed to cause an EINTR for it when it eventually gets sent.
• Somewhere in the loop, do a non-blocking waitpid(2), and handle its return appropriately. (Make sure you do this at least once after blocking SIGCHLD but before calling select for the first time, or you'll have a race condition.)