Fix flaky linking_interrupted test.

tests/testsuite/death.rs

@@ -8,52 +8,8 @@ use std::thread;
 
 use cargo_test_support::{project, slow_cpu_multiplier};
 
-#[cfg(unix)]
-fn enabled() -> bool {
-    true
-}
-
-// On Windows support for these tests is only enabled through the usage of job
-// objects. Support for nested job objects, however, was added in recent-ish
-// versions of Windows, so this test may not always be able to succeed.
-//
-// As a result, we try to add ourselves to a job object here
-// can succeed or not.
-#[cfg(windows)]
-fn enabled() -> bool {
-    use winapi::um::{handleapi, jobapi, jobapi2, processthreadsapi};
-
-    unsafe {
-        // If we're not currently in a job, then we can definitely run these
-        // tests.
-        let me = processthreadsapi::GetCurrentProcess();
-        let mut ret = 0;
-        let r = jobapi::IsProcessInJob(me, 0 as *mut _, &mut ret);
-        assert_ne!(r, 0);
-        if ret == ::winapi::shared::minwindef::FALSE {
-            return true;
-        }
-
-        // If we are in a job, then we can run these tests if we can be added to
-        // a nested job (as we're going to create a nested job no matter what as
-        // part of these tests.
-        //
-        // If we can't be added to a nested job, then these tests will
-        // definitely fail, and there's not much we can do about that.
-        let job = jobapi2::CreateJobObjectW(0 as *mut _, 0 as *const _);
-        assert!(!job.is_null());
-        let r = jobapi2::AssignProcessToJobObject(job, me);
-        handleapi::CloseHandle(job);
-        r != 0
-    }
-}
-
 #[cargo_test]
 fn ctrl_c_kills_everyone() {
-    if !enabled() {
-        return;
-    }
-
     let listener = TcpListener::bind("127.0.0.1:0").unwrap();
     let addr = listener.local_addr().unwrap();
 
@@ -132,14 +88,14 @@ fn ctrl_c_kills_everyone() {
 }
 
 #[cfg(unix)]
-fn ctrl_c(child: &mut Child) {
+pub fn ctrl_c(child: &mut Child) {
     let r = unsafe { libc::kill(-(child.id() as i32), libc::SIGINT) };
     if r < 0 {
         panic!("failed to kill: {}", io::Error::last_os_error());
     }
 }
 
 #[cfg(windows)]
-fn ctrl_c(child: &mut Child) {
+pub fn ctrl_c(child: &mut Child) {
     child.kill().unwrap();
 }

tests/testsuite/freshness.rs

@@ -10,6 +10,7 @@ use std::process::Stdio;
 use std::thread;
 use std::time::SystemTime;
 
+use super::death;
 use cargo_test_support::paths::{self, CargoPathExt};
 use cargo_test_support::registry::Package;
 use cargo_test_support::{basic_manifest, is_coarse_mtime, project, rustc_host, sleep_ms};
@@ -2316,8 +2317,14 @@ LLVM version: 9.0
 fn linking_interrupted() {
     // Interrupt during the linking phase shouldn't leave test executable as "fresh".
 
-    let listener = TcpListener::bind("127.0.0.1:0").unwrap();
-    let addr = listener.local_addr().unwrap();
+    // This is used to detect when linking starts, then to pause the linker so
+    // that the test can kill cargo.
+    let link_listener = TcpListener::bind("127.0.0.1:0").unwrap();
+    let link_addr = link_listener.local_addr().unwrap();
+
+    // This is used to detect when rustc exits.
+    let rustc_listener = TcpListener::bind("127.0.0.1:0").unwrap();
+    let rustc_addr = rustc_listener.local_addr().unwrap();
 
     // Create a linker that we can interrupt.
     let linker = project()
@@ -2326,8 +2333,6 @@ fn linking_interrupted() {
         .file(
             "src/main.rs",
             &r#"
-            use std::io::Read;
-
             fn main() {
                 // Figure out the output filename.
                 let output = match std::env::args().find(|a| a.starts_with("/OUT:")) {
@@ -2346,43 +2351,79 @@ fn linking_interrupted() {
                 std::fs::write(&output, "").unwrap();
                 // Tell the test that we are ready to be interrupted.
                 let mut socket = std::net::TcpStream::connect("__ADDR__").unwrap();
-                // Wait for the test to tell us to exit.
-                let _ = socket.read(&mut [0; 1]);
+                // Wait for the test to kill us.
+                std::thread::sleep(std::time::Duration::new(60, 0));
             }
             "#
-            .replace("__ADDR__", &addr.to_string()),
+            .replace("__ADDR__", &link_addr.to_string()),
         )
         .build();
     linker.cargo("build").run();
 
+    // Create a wrapper around rustc that will tell us when rustc is finished.
+    let rustc = project()
+        .at("rustc-waiter")
+        .file("Cargo.toml", &basic_manifest("rustc-waiter", "1.0.0"))
+        .file(
+            "src/main.rs",
+            &r#"
+            fn main() {
+                let mut conn = None;
+                // Check for a normal build (not -vV or --print).
+                if std::env::args().any(|arg| arg == "t1") {
+                    // Tell the test that rustc has started.
+                    conn = Some(std::net::TcpStream::connect("__ADDR__").unwrap());
+                }
+                let status = std::process::Command::new("rustc")
+                    .args(std::env::args().skip(1))
+                    .status()
+                    .expect("rustc to run");
+                std::process::exit(status.code().unwrap_or(1));
+            }
+            "#
+            .replace("__ADDR__", &rustc_addr.to_string()),
+        )
+        .build();
+    rustc.cargo("build").run();
+
     // Build it once so that the fingerprint gets saved to disk.
     let p = project()
         .file("src/lib.rs", "")
         .file("tests/t1.rs", "")
         .build();
     p.cargo("test --test t1 --no-run").run();
+
     // Make a change, start a build, then interrupt it.
     p.change_file("src/lib.rs", "// modified");
     let linker_env = format!(
         "CARGO_TARGET_{}_LINKER",
         rustc_host().to_uppercase().replace('-', "_")
     );
+    // NOTE: This assumes that the paths to the linker or rustc are not in the
+    // fingerprint. But maybe they should be?
     let mut cmd = p
         .cargo("test --test t1 --no-run")
         .env(&linker_env, linker.bin("linker"))
+        .env("RUSTC", rustc.bin("rustc-waiter"))
         .build_command();
     let mut child = cmd
         .stdout(Stdio::null())
         .stderr(Stdio::null())
+        .env("__CARGO_TEST_SETSID_PLEASE_DONT_USE_ELSEWHERE", "1")
         .spawn()
         .unwrap();
+    // Wait for rustc to start.
+    let mut rustc_conn = rustc_listener.accept().unwrap().0;
     // Wait for linking to start.
-    let mut conn = listener.accept().unwrap().0;
+    drop(link_listener.accept().unwrap());
 
     // Interrupt the child.
-    child.kill().unwrap();
-    // Note: rustc and the linker are still running, let them exit here.
-    conn.write(b"X").unwrap();
+    death::ctrl_c(&mut child);
+    assert!(!child.wait().unwrap().success());
+    // Wait for rustc to exit. If we don't wait, then the command below could
+    // start while rustc is still being torn down.
+    let mut buf = [0];
+    drop(rustc_conn.read_exact(&mut buf));
 
     // Build again, shouldn't be fresh.
     p.cargo("test --test t1")