[Crash-utility] crash CPU bound waiting for user response

[Dave Anderson]

D. Hugh Redelmeier wrote:
> | From: Dave Anderson <anderson at redhat.com>
> 
> | Yeah, but I'm still paranoid about re-introducing the hang I used
> | to see.
> | 
> | I think I'll just put a "stall(1000);" call after each waitpid().
> | Works for me...
> 
> I take that as a challenge. I think that I've eliminated this
> busy-wait and simplified the code.  I admit that wait_for_children is
> slightly more complicated.
> 
> In light testing this worked fine.  I'd really like to find out how
> this is going to hang.  I promise to attempt to debug any hang that I
> can duplicate.
> 

I didn't mean it as a challenge.  I appreciate the effort, but
at this point I'm really not particularly interested in re-working
this area, especially since it can be so simply avoided with a stall.

Thanks,
   Dave


> ===================================================================
> RCS file: RCS/cmdline.c,v
> retrieving revision 1.1
> diff -u -r1.1 cmdline.c
> --- cmdline.c	2007/07/10 19:30:01	1.1
> +++ cmdline.c	2007/07/10 20:16:57
> @@ -31,6 +31,7 @@
>  static void set_my_tty(void);
>  static char *signame(int);
>  static int setup_stdpipe(void);
> +static void wait_for_child(pid_t *);
>  static void wait_for_children(ulong);
>  #define ZOMBIES_ONLY (1)
>  #define ALL_CHILDREN (2)
> @@ -878,25 +879,15 @@
>          if (pc->stdpipe) {
>  		close(fileno(pc->stdpipe));
>                  pc->stdpipe = NULL;
> -		if (pc->stdpipe_pid && PID_ALIVE(pc->stdpipe_pid)) {
> -			while (!waitpid(pc->stdpipe_pid, &waitstatus, WNOHANG))
> -				;
> -		}
> -		pc->stdpipe_pid = 0;
> +		wait_for_child(&pc->stdpipe_pid);
>          }
>  	if (pc->pipe) {
>  		close(fileno(pc->pipe));
>  	 	pc->pipe = NULL;
>  		console("wait for redirect %d->%d to finish...\n",
>  			pc->pipe_shell_pid, pc->pipe_pid);
> -		if (pc->pipe_pid)
> -			while (PID_ALIVE(pc->pipe_pid)) 
> -				waitpid(pc->pipe_pid, &waitstatus, WNOHANG);
> -                if (pc->pipe_shell_pid)
> -		        while (PID_ALIVE(pc->pipe_shell_pid)) 
> -                        	waitpid(pc->pipe_shell_pid, 
> -					&waitstatus, WNOHANG);
> -		pc->pipe_pid = 0;
> +		wait_for_child(&pc->pipe_pid);
> +		wait_for_child(&pc->pipe_shell_pid);
>  	}
>  	if (pc->ifile_pipe) {
>  		fflush(pc->ifile_pipe);
> @@ -907,12 +898,8 @@
>                      (FROM_INPUT_FILE|REDIRECT_TO_PIPE|REDIRECT_PID_KNOWN))) {
>  			console("wait for redirect %d->%d to finish...\n",
>  				pc->pipe_shell_pid, pc->pipe_pid);
> -                	while (PID_ALIVE(pc->pipe_pid))
> -				waitpid(pc->pipe_pid, &waitstatus, WNOHANG);
> -                        if (pc->pipe_shell_pid) 
> -                                while (PID_ALIVE(pc->pipe_shell_pid))
> -                                        waitpid(pc->pipe_shell_pid,
> -                                                &waitstatus, WNOHANG);
> +		        wait_for_child(&pc->pipe_pid);
> +			wait_for_child(&pc->pipe_shell_pid);
>  			if (pc->redirect & (REDIRECT_MULTI_PIPE))
>  				wait_for_children(ALL_CHILDREN);
>  		}
> @@ -1967,7 +1954,7 @@
>  
>  		if (CRASHDEBUG(2))
>  			console("pipe: %lx\n", pc->stdpipe);
> -		return TRUE;;
> +		return TRUE;
>  
>  	} else {                        
>  		close(pc->pipefd[1]);    /* child closes write end */
> @@ -1998,13 +1985,26 @@
>  	}
>  }
>  
> +static void
> +wait_for_child(pid_t *chp)
> +{
> +	if (*chp != 0)
> +	{
> +		int waitstatus;
> +
> +		do ; while (waitpid(*chp, &waitstatus, 0) == -1 && errno == EINTR);
> +		*chp = 0;
> +	}
> +}
> +
>  static void 
>  wait_for_children(ulong waitflag)
>  {
> -        int status, pid;
> -
>  	while (TRUE) {
> -        	switch (pid = waitpid(-1, &status, WNOHANG))
> +		int status;
> +		pid_t pid = waitpid(-1, &status, waitflag==ZOMBIES_ONLY? WNOHANG : 0);
> +
> +        	switch (pid)
>          	{
>          	case  0:
>  			if (CRASHDEBUG(2))
> @@ -2014,6 +2014,8 @@
>  			break;
>  
>          	case -1:
> +			if (errno == EINTR)
> +			    continue;
>  			if (CRASHDEBUG(2))
>  			    console("wait_for_children: no children alive\n");
>                  	return;
> ================ end ================
> 
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility