2

I'm executing the code below and the call to waitpid() always returns -1, thus the code bellow ends with an infinite loop. The call works if I replace WNOHANG with 0.

void execute(cmdLine* pCmdLine) {
    int status = 0;
    pid_t pid = fork();
    if(pid == 0) {
         if(execvp(pCmdLine->arguments[0], pCmdLine->arguments) == -1) {
             if(strcmp(pCmdLine->arguments[0], "cd") != 0) {
               perror("execute failed\n");
             }
        _exit(1);
        }
    } else {
        if(pCmdLine->blocking == 1) {
            waitpid(pid, &status, 0);
        }
            while(waitpid(pid, &status, WNOHANG) == -1) {
             printf("still -1\n");
            }
         }     
    }
}
S.S. Anne
  • 15,171
  • 8
  • 38
  • 76
Liavba
  • 75
  • 9
  • You really don't have enough diagnostics in the code — test the return value of the first `waitpid()` and print it. Check the `errno` for the `while` loop; print it and the error message. Don't use `WNOHANG` when you want to wait for the child to die. – Jonathan Leffler Apr 23 '19 at 05:22

2 Answers2

2

Well, you have misunderstood the workings of the wait system call.

As with malloc/free, you can only successfully waitpid() only once per fork()ed process... so the while loop is never necessary if you are going to wait for the exit code of the child, you have to call it only once. Wait will only return -1 in your case because of two reasons:

  • fork() didn't succeed, so you are waiting for an invalid pid. Indeed, you should be calling wait() for pid == -1, which is invalid. In case you wait() and there's no process to be waited for (in case the pid variable has a positive number, but of an already wait()ed subprocess, you also get -1), you get an error from any of the wait() family of system calls. The mission of zombie processes in UN*X systems is just this, to ensure that a wait() for an already finished child is still valid and the calling process gets the exit code signalled by the child on exit().
  • You expressely say you are not going to wait for the process to finish. It should be clear that if you are not going to wait for the process to terminate, this is what you are doing with the WNOHANG parameter, then the child process can be still running (which is your case) and had not yet done an exit() syscall. You only want the exit code in case the child process has already finished. If this is the case, then you had better to write:

    while(waitpid(pid, &status, WNOHANG) == -1 && errno == EAGAIN)
        do_whatever_you_want_because_you_decided_not_to_wait();
    

    The wait system call has no way to tell you that the &status variable has not been filled with the exit code of the child process than signalling an error, and in that case, it always sets errno to EAGAIN.

    but, from my point of view, if you have nothing to do in the meanwhile, then you had better not to use WNOHANG. That will save cpu cycles and a lot of heat energy thrown to the environment.

Luis Colorado
  • 10,974
  • 1
  • 16
  • 31
1

Here

 while(waitpid(pid,&status,WNOHANG)==-1) { }

when if there is no more child process exists then waitpid returns -1 and it makes while(true) always and that cause infinite loop.

From the manual page of waitpid().

waitpid(): on success, returns the process ID of the child whose state has changed; if WNOHANG was specified and one or more child(ren) specified by pid exist, but have not yet changed state, then 0 is returned. On error, -1 is returned.

That means, when there are no more child to wait for, it returns -1. So either make it like

if() { /* child process. can be multiple */
} 
else { /* parent process */
    while(waitpid(pid,&status,WNOHANG) != -1) { /* when there is no more child process exists then it terminate */ 
    }
}

or

if() { /* child process. can be multiple */
} 
else { /* parent process */
  while(waitpid(pid,&status,WNOHANG) == -1);  /* dummy while ..when there is no more child process exists then it terminate */ 
}
Achal
  • 11,821
  • 2
  • 15
  • 37