Rollback the (functional) effect of 13944 and 14134

Re-opening the FIFO before closing it gives (difficult to understand)
problems => rollback the change that keeps the FIFO opened.
Rather handle the race condition by retrying at vgdb side.
See extensive comments in remote-utils.c

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@14147

diff --git a/coregrind/m_gdbserver/remote-utils.c b/coregrind/m_gdbserver/remote-utils.c
index 042ad44faf4418efb539cb84fcf1884a6a4b0d18..3b23600eeea2569d6cd375784904d9ad9e215fff 100644 (file)
--- a/coregrind/m_gdbserver/remote-utils.c
+++ b/coregrind/m_gdbserver/remote-utils.c
@@ -463,25 +463,31 @@ void remote_finish (FinishReason reason)
    
    if (remote_desc != INVALID_DESCRIPTOR) {
       /* Fully close the connection, either due to orderly_finish or
-         to reset_after_fork.
-         For reset_after_error, we must always keep the reading side open,
-         to always be ready to accept new vgdb connection. So, we first
-         re-open the FIFO before closing the currently opened fd. */
-      if (reason == reset_after_error) {
-         /* Save current remote_desc, and set it to invalid, so that
-            setup_remote_desc_for_reading does (re-)open the read FIFO side. */
-         int save_remote_desc = remote_desc;
-         remote_desc = INVALID_DESCRIPTOR;
-         setup_remote_desc_for_reading();
-         VG_(close) (save_remote_desc);
-      } else {
-         vg_assert (reason == reset_after_fork || reason == orderly_finish);
-         remote_desc_pollfdread_activity.fd = INVALID_DESCRIPTOR;
-         remote_desc_pollfdread_activity.events = 0;
-         remote_desc_pollfdread_activity.revents = 0;
-         VG_(close) (remote_desc);
-         remote_desc = INVALID_DESCRIPTOR;
-      }
+         to reset_after_fork or reset_after_error.  For
+         reset_after_error, the FIFO will be re-opened soon.  This
+         leaves a small window during which a race condition can
+         happen between vgdb and a forking process: Just after fork,
+         both the parent and the child have the FIFO open.  The child
+         will close it asap (as part of the 'after fork cleanup').  If
+         2 vgdbs are launched very quickly just after the fork, the
+         parent will close its FIFO when the 1st vgdb exits.  Then if
+         the 2nd vgdb is started before the parent has the time to
+         re-open the FIFO, the 2nd vgdb will be able to open the FIFO
+         (as it is still opened by the child).  The 2nd vgdb can then
+         have a 'write' error when the child closes the FIFO.  After
+         the 1st vgdb closes its FIFO write side, the parent gets EOF
+         on its reading FIFO till it is closed and re-opened.  Opening
+         a 2nd time the FIFO before closing the 'previous fd' solves
+         this race condition, but causes other (not understood)
+         problems due to too early re-invocation of gdbsrv.  Rather
+         than to handle this race condition in gdbsrv side, we put a
+         'retry' loop in vgdb for the initial write on the write
+         FIFO. */
+      remote_desc_pollfdread_activity.fd = INVALID_DESCRIPTOR;
+      remote_desc_pollfdread_activity.events = 0;
+      remote_desc_pollfdread_activity.revents = 0;
+      VG_(close) (remote_desc);
+      remote_desc = INVALID_DESCRIPTOR;
    }
    noack_mode = False;
    

diff --git a/coregrind/vgdb.c b/coregrind/vgdb.c
index fc17291336c5a005ae935aff4f42a47ce1fccf44..d330cb5d84cb1d43d6b32b44df442473573d6965 100644 (file)
--- a/coregrind/vgdb.c
+++ b/coregrind/vgdb.c
@@ -958,7 +958,17 @@ void standalone_send_commands(int pid,
       We then start to wait for packets (normally first a resume reply)
       At that point, we send our command and expect replies */
    buf[0] = '\003';
-   write_buf(to_pid, buf, 1, "write \\003 to wake up", /* notify */ True);
+   i = 0;
+   while (!write_buf(to_pid, buf, 1, 
+                     "write \\003 to wake up", /* notify */ True)) {
+      /* If write fails, retries up to 10 times every 0.5 seconds
+         This aims at solving the race condition described in
+         remote-utils.c remote_finish function. */
+      usleep(500*1000);
+      i++;
+      if (i >= 10)
+         XERROR (errno, "failed to send wake up char after 10 trials\n");
+   }
    from_pid = open_fifo(to_gdb_from_pid, O_RDONLY, 
                         "read cmd result from pid");