Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Terry Guo" <terry.guo@arm.com>
To: <gdb-patches@sourceware.org>
Cc: <eliz@gnu.org>,	"Joey Ye" <Joey.Ye@arm.com>,
	"Matthew Gretton-Dann" <Matthew.Gretton-Dann@arm.com>,
	"'Pedro Alves'" <palves@redhat.com>,
	<daniel.jacobowitz@gmail.com>
Subject: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub
Date: Tue, 26 Jun 2012 01:12:00 -0000	[thread overview]
Message-ID: <000001cd5338$ded61b20$9c825160$@guo@arm.com> (raw)

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;



             reply	other threads:[~2012-06-26  1:12 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-26  1:12 Terry Guo [this message]
2012-06-27  1:33 ` Terry Guo
     [not found] <000001cd5338$ded61b20$9c825160$%guo@arm.com>
     [not found] ` <000501cd5404$fbf210c0$f3d63240$%guo@arm.com>
2012-06-27  3:06   ` Eli Zaretskii
2012-06-27 16:54 ` Eli Zaretskii
2012-07-04  8:19   ` Terry Guo
2012-07-11  5:00     ` Terry Guo
     [not found]   ` <000301cd59bd$ce1c8900$6a559b00$%guo@arm.com>
     [not found]     ` <000101cd5f22$2371cdc0$6a556940$%guo@arm.com>
2012-07-11 18:36       ` Eli Zaretskii
2012-07-12  1:41         ` Terry Guo
     [not found]         ` <000301cd5fcf$838d31b0$8aa79510$%guo@arm.com>
2012-07-12  5:20           ` Eli Zaretskii
2012-07-16  5:51             ` Terry Guo
2012-07-17  1:28               ` Sergio Durigan Junior
2012-07-17  3:05                 ` Terry Guo
2012-07-17  7:51                 ` Terry Guo
2012-07-17 17:21                   ` Sergio Durigan Junior
2012-07-16  6:08             ` Terry Guo
2012-07-16  6:38             ` Yao Qi
2012-07-16  6:55               ` Terry Guo
2012-07-16  7:19               ` Yao Qi
2012-07-16  8:10                 ` Terry Guo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='000001cd5338$ded61b20$9c825160$@guo@arm.com' \
    --to=terry.guo@arm.com \
    --cc=Joey.Ye@arm.com \
    --cc=Matthew.Gretton-Dann@arm.com \
    --cc=daniel.jacobowitz@gmail.com \
    --cc=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox