RE: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

["Terry Guo" <terry.guo@arm.com>]

Hi Eli,

Can you help to review this proposal? The GDB trunk also has this issue.
Thanks.

BR,
Terry

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Terry Guo
> Sent: Tuesday, June 26, 2012 9:13 AM
> To: gdb-patches@sourceware.org
> Cc: eliz@gnu.org; Joey Ye; Matthew Gretton-Dann; 'Pedro Alves';
> daniel.jacobowitz@gmail.com
> Subject: [PATCH]Fix that GDB will get hang on Windows when using pipe
> to get stdout and stderr from stub
> 
> Hi,
> 
> I noticed a cross-built MINGW arm-none-eabi GDB will get hang on
> Windows
> when use pipe to get stderr and stdout from stub. The command used to
> start
> stub in GDB is "target extended-remote |
> stub-that-write-stderr-before-stdout". For my case, after send
> "$vFlashDone#ea" to stub, GDB get hang. The GDB source show that GDB
> will
> keep waiting for ACK message from stdout of stub, after send the packet.
> Unfortunately my stub will write some kind of log information into
> stderr
> and this action takes place before stub write ACK message to its stdout.
> So
> the only pipe is occupied by stderr which is waiting for GDB to consume,
> while GDB keep waiting for message from the stdout which hasn't pipe to
> use.
> We finally end up with a deadlock on pipe between GDB/stderr/stdout.
> 
> The following patch can avoid such deadlock by letting GDB also probe
> and
> consume stderr when waiting for stdout. Please review and comment.
> 
> The Linux version GDB hasn't such issue. I think it's because we use
> different way to handle PIPE as stated in functions pipe_open and
> pipe_windows_open. For Linux we have two socketpair kind pipes, one for
> stdout and one for stderr. While for windows, we only have one pipe
> which is
> created by _pipe function.
> 
> BR,
> Terry
> 
> 2012-06-25  Terry Guo  <terry.guo@arm.com>
> 
> 	* ser_base (ser_base_read_error_fd): New function.
> 	(do_ser_base_readchar): Poll error file descriptor as well as
> 	standard output.
> 	(generic_readchar): Refactor error handling.
> 
> diff --git a/gdb/ser-base.c b/gdb/ser-base.c
> index 368afa6..ee6db54 100644
> --- a/gdb/ser-base.c
> +++ b/gdb/ser-base.c
> @@ -223,6 +223,63 @@ ser_base_wait_for (struct serial *scb, int timeout)
>      }
>  }
> 
> +/* Read any error output we might have.  */
> +
> +void
> +ser_base_read_error_fd (struct serial *scb, int close_fd)
> +{
> +  if (scb->error_fd != -1)
> +    {
> +      ssize_t s;
> +      char buf[81];
> +
> +      for (;;)
> +        {
> + 	  char *current;
> + 	  char *newline;
> +	  int to_read = 80;
> +	  int num_bytes = -1;
> +
> +	  if (scb->ops->avail)
> +	    num_bytes = (scb->ops->avail)(scb, scb->error_fd);
> +
> +	  if (num_bytes != -1)
> +	    to_read = (num_bytes < to_read) ? num_bytes : to_read;
> +
> +	  if (to_read == 0)
> +	    break;
> +
> +	  s = read (scb->error_fd, &buf, to_read);
> +	  if ((s == -1) || (s == 0 && !close_fd))
> +	    break;
> +
> +	  if (s == 0 && close_fd)
> +	    {
> +	      /* End of file.  */
> +	      close (scb->error_fd);
> +	      scb->error_fd = -1;
> +	      break;
> +	    }
> +
> +	  /* In theory, embedded newlines are not a problem.
> +	     But for MI, we want each output line to have just
> +	     one newline for legibility.  So output things
> +	     in newline chunks.  */
> +	  buf[s] = '\0';
> +	  current = buf;
> +	  while ((newline = strstr (current, "\n")) != NULL)
> +	    {
> +	      *newline = '\0';
> +	      fputs_unfiltered (current, gdb_stderr);
> +	      fputs_unfiltered ("\n", gdb_stderr);
> +	      current = newline + 1;
> +	    }
> +
> +	  fputs_unfiltered (current, gdb_stderr);
> +	}
> +    }
> +}
> +
>  /* Read a character with user-specified timeout.  TIMEOUT is number of
> seconds
>     to wait, or -1 to wait forever.  Use timeout of 0 to effect a poll.
> Returns
>     char if successful.  Returns -2 if timeout expired, EOF if line
> dropped
> @@ -273,6 +330,11 @@ do_ser_base_readchar (struct serial *scb, int
> timeout)
>  	  status = SERIAL_TIMEOUT;
>  	  break;
>  	}
> +
> +      /* We also need to check and consume the stderr because it could
> +         come before the stdout for some stubs.  If we just sit and
> wait
> +         for stdout, we would hit a deadlock for that case.  */
> +      ser_base_read_error_fd (scb, 0);
>      }
> 
>    if (status < 0)
> @@ -344,53 +406,7 @@ generic_readchar (struct serial *scb, int timeout,
>  	}
>      }
>    /* Read any error output we might have.  */
> -  if (scb->error_fd != -1)
> -    {
> -      ssize_t s;
> -      char buf[81];
> -
> -      for (;;)
> -        {
> - 	  char *current;
> - 	  char *newline;
> -	  int to_read = 80;
> -
> -	  int num_bytes = -1;
> -	  if (scb->ops->avail)
> -	    num_bytes = (scb->ops->avail)(scb, scb->error_fd);
> -	  if (num_bytes != -1)
> -	    to_read = (num_bytes < to_read) ? num_bytes : to_read;
> -
> -	  if (to_read == 0)
> -	    break;
> -
> -	  s = read (scb->error_fd, &buf, to_read);
> -	  if (s == -1)
> -	    break;
> -	  if (s == 0)
> -	    {
> -	      /* EOF */
> -	      close (scb->error_fd);
> -	      scb->error_fd = -1;
> -	      break;
> -	    }
> -
> -	  /* In theory, embedded newlines are not a problem.
> -	     But for MI, we want each output line to have just
> -	     one newline for legibility.  So output things
> -	     in newline chunks.  */
> -	  buf[s] = '\0';
> -	  current = buf;
> -	  while ((newline = strstr (current, "\n")) != NULL)
> -	    {
> -	      *newline = '\0';
> -	      fputs_unfiltered (current, gdb_stderr);
> -	      fputs_unfiltered ("\n", gdb_stderr);
> -	      current = newline + 1;
> -	    }
> -	  fputs_unfiltered (current, gdb_stderr);
> -	}
> -    }
> +  ser_base_read_error_fd (scb, 1);
> 
>    reschedule (scb);
>    return ch;
> 
>