Auto merge of #38883 - alexcrichton:android-flaky, r=brson

compiletest: Fix flaky Android gdb test runs

Local testing showed that I was able to reproduce an error where debuginfo tests
on Android would fail with "connection reset by peer". Further investigation
turned out that the gdb tests are android with bit of process management:

* First an `adb forward` command is run to ensure that the host's port 5039 is
  the same as the emulator's.
* Next an `adb shell` command is run to execute the `gdbserver` executable
  inside the emulator. This gdb server will attach to port 5039 and listen for
  remote gdb debugging sessions.
* Finally, we run `gdb` on the host (not in the emulator) and then connect to
  this gdb server to send it commands.

The problem was happening when the host's gdb was failing to connect to the
remote gdbserver running inside the emulator. The previous test for this was
that after `adb shell` executed we'd sleep for a second and then attempt to make
a TCP connection to port 5039. If successful we'd run gdb and on failure we'd
sleep again.

It turns out, however, that as soon as we've executed `adb forward` all TCP
connections to 5039 will succeed. This means that we would only ever sleep for
at most one second, and if this wasn't enough time we'd just fail later because
we would assume that gdbserver had started but it may not have done so yet.

This commit fixes these issues by removing the TCP connection to test if
gdbserver is ready to go. Instead we read the stdout of the process and wait for
it to print that it's listening at which point we start running gdb. I've found
that locally at least I was unable to reproduce the failure after these changes.

Closes #38710

src/tools/compiletest/src/procsrv.rs

@@ -11,6 +11,7 @@
 use std::env;
 use std::ffi::OsString;
 use std::io::prelude::*;
+use std::io;
 use std::path::PathBuf;
 use std::process::{Child, Command, ExitStatus, Output, Stdio};
 
@@ -52,7 +53,7 @@ pub fn run(lib_path: &str,
            args: &[String],
            env: Vec<(String, String)>,
            input: Option<String>)
-           -> Option<Result> {
+           -> io::Result<Result> {
 
     let mut cmd = Command::new(prog);
     cmd.args(args)
@@ -64,21 +65,17 @@ pub fn run(lib_path: &str,
         cmd.env(&key, &val);
     }
 
-    match cmd.spawn() {
-        Ok(mut process) => {
-            if let Some(input) = input {
-                process.stdin.as_mut().unwrap().write_all(input.as_bytes()).unwrap();
-            }
-            let Output { status, stdout, stderr } = process.wait_with_output().unwrap();
-
-            Some(Result {
-                status: status,
-                out: String::from_utf8(stdout).unwrap(),
-                err: String::from_utf8(stderr).unwrap(),
-            })
-        }
-        Err(..) => None,
+    let mut process = cmd.spawn()?;
+    if let Some(input) = input {
+        process.stdin.as_mut().unwrap().write_all(input.as_bytes()).unwrap();
     }
+    let Output { status, stdout, stderr } = process.wait_with_output().unwrap();
+
+    Ok(Result {
+        status: status,
+        out: String::from_utf8(stdout).unwrap(),
+        err: String::from_utf8(stderr).unwrap(),
+    })
 }
 
 pub fn run_background(lib_path: &str,
@@ -87,26 +84,21 @@ pub fn run_background(lib_path: &str,
                       args: &[String],
                       env: Vec<(String, String)>,
                       input: Option<String>)
-                      -> Option<Child> {
+                      -> io::Result<Child> {
 
     let mut cmd = Command::new(prog);
     cmd.args(args)
-        .stdin(Stdio::piped())
-        .stdout(Stdio::piped())
-        .stderr(Stdio::piped());
+       .stdin(Stdio::piped())
+       .stdout(Stdio::piped());
     add_target_env(&mut cmd, lib_path, aux_path);
     for (key, val) in env {
         cmd.env(&key, &val);
     }
 
-    match cmd.spawn() {
-        Ok(mut process) => {
-            if let Some(input) = input {
-                process.stdin.as_mut().unwrap().write_all(input.as_bytes()).unwrap();
-            }
-
-            Some(process)
-        }
-        Err(..) => None,
+    let mut process = cmd.spawn()?;
+    if let Some(input) = input {
+        process.stdin.as_mut().unwrap().write_all(input.as_bytes()).unwrap();
     }
+
+    Ok(process)
 }

src/tools/compiletest/src/runtest.rs

@@ -21,13 +21,12 @@ use test::TestPaths;
 use uidiff;
 use util::logv;
 
-use std::env;
 use std::collections::HashSet;
+use std::env;
 use std::fmt;
 use std::fs::{self, File};
-use std::io::{self, BufReader};
 use std::io::prelude::*;
-use std::net::TcpStream;
+use std::io::{self, BufReader};
 use std::path::{Path, PathBuf};
 use std::process::{Command, Output, ExitStatus};
 use std::str;
@@ -506,8 +505,8 @@ actual:\n\
                                  exe_file.to_str().unwrap().to_owned(),
                                  self.config.adb_test_dir.clone()
                              ],
-                             vec![("".to_owned(), "".to_owned())],
-                             Some("".to_owned()))
+                             Vec::new(),
+                             None)
                     .expect(&format!("failed to exec `{:?}`", self.config.adb_path));
 
                 procsrv::run("",
@@ -518,8 +517,8 @@ actual:\n\
                                  "tcp:5039".to_owned(),
                                  "tcp:5039".to_owned()
                              ],
-                             vec![("".to_owned(), "".to_owned())],
-                             Some("".to_owned()))
+                             Vec::new(),
+                             None)
                     .expect(&format!("failed to exec `{:?}`", self.config.adb_path));
 
                 let adb_arg = format!("export LD_LIBRARY_PATH={}; \
@@ -539,17 +538,23 @@ actual:\n\
                                                               "shell".to_owned(),
                                                               adb_arg.clone()
                                                           ],
-                                                          vec![("".to_owned(),
-                                                                "".to_owned())],
-                                                          Some("".to_owned()))
+                                                          Vec::new(),
+                                                          None)
                     .expect(&format!("failed to exec `{:?}`", self.config.adb_path));
+
+                // Wait for the gdbserver to print out "Listening on port ..."
+                // at which point we know that it's started and then we can
+                // execute the debugger below.
+                let mut stdout = BufReader::new(process.stdout.take().unwrap());
+                let mut line = String::new();
                 loop {
-                    //waiting 1 second for gdbserver start
-                    ::std::thread::sleep(::std::time::Duration::new(1,0));
-                    if TcpStream::connect("127.0.0.1:5039").is_ok() {
+                    line.truncate(0);
+                    stdout.read_line(&mut line).unwrap();
+                    if line.starts_with("Listening on port 5039") {
                         break
                     }
                 }
+                drop(stdout);
 
                 let debugger_script = self.make_out_name("debugger.script");
                 // FIXME (#9639): This needs to handle non-utf8 paths
@@ -569,7 +574,7 @@ actual:\n\
                                  &gdb_path,
                                  None,
                                  &debugger_opts,
-                                 vec![("".to_owned(), "".to_owned())],
+                                 Vec::new(),
                                  None)
                     .expect(&format!("failed to exec `{:?}`", gdb_path));
                 let cmdline = {