Skip to content

Commit bb7ec2f

Browse files
committed
Fix hang on broken stderr.
1 parent 8eb0e9a commit bb7ec2f

File tree

2 files changed

+97
-20
lines changed

2 files changed

+97
-20
lines changed

src/cargo/core/compiler/job_queue.rs

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ struct DrainState<'cfg> {
149149
pending_queue: Vec<(Unit, Job)>,
150150
print: DiagnosticPrinter<'cfg>,
151151

152-
// How many jobs we've finished
152+
/// How many jobs we've finished
153153
finished: usize,
154154
}
155155

@@ -469,7 +469,15 @@ impl<'cfg> DrainState<'cfg> {
469469
// we're able to perform some parallel work.
470470
while self.has_extra_tokens() && !self.pending_queue.is_empty() {
471471
let (unit, job) = self.pending_queue.remove(0);
472-
self.run(&unit, job, cx, scope)?;
472+
*self.counts.get_mut(&unit.pkg.package_id()).unwrap() -= 1;
473+
if !cx.bcx.build_config.build_plan {
474+
// Print out some nice progress information.
475+
// NOTE: An error here will drop the job without starting it.
476+
// That should be OK, since we want to exit as soon as
477+
// possible during an error.
478+
self.note_working_on(cx.bcx.config, &unit, job.freshness())?;
479+
}
480+
self.run(&unit, job, cx, scope);
473481
}
474482

475483
Ok(())
@@ -835,31 +843,22 @@ impl<'cfg> DrainState<'cfg> {
835843
}
836844
}
837845

838-
/// Executes a job, pushing the spawned thread's handled onto `threads`.
839-
fn run(
840-
&mut self,
841-
unit: &Unit,
842-
job: Job,
843-
cx: &Context<'_, '_>,
844-
scope: &Scope<'_>,
845-
) -> CargoResult<()> {
846+
/// Executes a job.
847+
///
848+
/// Fresh jobs block until finished (which should be very fast!), Dirty
849+
/// jobs will spawn a thread in the background and return immediately.
850+
fn run(&mut self, unit: &Unit, job: Job, cx: &Context<'_, '_>, scope: &Scope<'_>) {
846851
let id = JobId(self.next_id);
847852
self.next_id = self.next_id.checked_add(1).unwrap();
848853

849854
info!("start {}: {:?}", id, unit);
850855

851856
assert!(self.active.insert(id, unit.clone()).is_none());
852-
*self.counts.get_mut(&unit.pkg.package_id()).unwrap() -= 1;
853857

854858
let messages = self.messages.clone();
855859
let fresh = job.freshness();
856860
let rmeta_required = cx.rmeta_required(unit);
857861

858-
if !cx.bcx.build_config.build_plan {
859-
// Print out some nice progress information.
860-
self.note_working_on(cx.bcx.config, unit, fresh)?;
861-
}
862-
863862
let doit = move |state: JobState<'_>| {
864863
let mut sender = FinishOnDrop {
865864
messages: &state.messages,
@@ -934,8 +933,6 @@ impl<'cfg> DrainState<'cfg> {
934933
});
935934
}
936935
}
937-
938-
Ok(())
939936
}
940937

941938
fn emit_warnings(

tests/testsuite/build.rs

Lines changed: 82 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@ use cargo::{
1010
use cargo_test_support::paths::{root, CargoPathExt};
1111
use cargo_test_support::registry::Package;
1212
use cargo_test_support::{
13-
basic_bin_manifest, basic_lib_manifest, basic_manifest, git, is_nightly, lines_match_unordered,
14-
main_file, paths, project, rustc_host, sleep_ms, symlink_supported, t, Execs, ProjectBuilder,
13+
basic_bin_manifest, basic_lib_manifest, basic_manifest, cargo_exe, git, is_nightly,
14+
lines_match_unordered, main_file, paths, process, project, rustc_host, sleep_ms,
15+
symlink_supported, t, Execs, ProjectBuilder,
1516
};
1617
use std::env;
1718
use std::fs;
@@ -5256,6 +5257,85 @@ hello stderr!
52565257
lines_match_unordered("hello stdout!\n", &stdout).unwrap();
52575258
}
52585259

5260+
#[cargo_test]
5261+
fn close_output_during_drain() {
5262+
// Test to close the output during the build phase (drain_the_queue).
5263+
// There was a bug where it would hang.
5264+
5265+
// Server to know when rustc has spawned.
5266+
let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
5267+
let addr = listener.local_addr().unwrap();
5268+
5269+
// Create a wrapper so the test can know when compiling has started.
5270+
let rustc_wrapper = {
5271+
let p = project()
5272+
.at("compiler")
5273+
.file("Cargo.toml", &basic_manifest("compiler", "1.0.0"))
5274+
.file(
5275+
"src/main.rs",
5276+
&r#"
5277+
use std::process::Command;
5278+
use std::env;
5279+
use std::io::Read;
5280+
5281+
fn main() {
5282+
// Only wait on the first dependency.
5283+
if matches!(env::var("CARGO_PKG_NAME").as_deref(), Ok("dep")) {
5284+
let mut socket = std::net::TcpStream::connect("__ADDR__").unwrap();
5285+
// Wait for the test to tell us to start printing.
5286+
let mut buf = [0];
5287+
drop(socket.read_exact(&mut buf));
5288+
}
5289+
let mut cmd = Command::new("rustc");
5290+
for arg in env::args_os().skip(1) {
5291+
cmd.arg(arg);
5292+
}
5293+
std::process::exit(cmd.status().unwrap().code().unwrap());
5294+
}
5295+
"#
5296+
.replace("__ADDR__", &addr.to_string()),
5297+
)
5298+
.build();
5299+
p.cargo("build").run();
5300+
p.bin("compiler")
5301+
};
5302+
5303+
Package::new("dep", "1.0.0").publish();
5304+
let p = project()
5305+
.file(
5306+
"Cargo.toml",
5307+
r#"
5308+
[package]
5309+
name = "foo"
5310+
version = "0.1.0"
5311+
5312+
[dependencies]
5313+
dep = "1.0"
5314+
"#,
5315+
)
5316+
.file("src/lib.rs", "")
5317+
.build();
5318+
5319+
// Spawn cargo, wait for the first rustc to start, and then close stderr.
5320+
let mut cmd = process(&cargo_exe())
5321+
.arg("check")
5322+
.cwd(p.root())
5323+
.env("RUSTC", rustc_wrapper)
5324+
.build_command();
5325+
cmd.stdout(Stdio::piped()).stderr(Stdio::piped());
5326+
let mut child = cmd.spawn().expect("cargo should spawn");
5327+
// Wait for the rustc wrapper to start.
5328+
let rustc_conn = listener.accept().unwrap().0;
5329+
// Close stderr to force an error.
5330+
drop(child.stderr.take());
5331+
// Tell the wrapper to continue.
5332+
drop(rustc_conn);
5333+
match child.wait() {
5334+
Ok(status) => assert!(!status.success()),
5335+
Err(e) => panic!("child wait failed: {}", e),
5336+
}
5337+
}
5338+
52595339
use cargo_test_support::registry::Dependency;
52605340

52615341
#[cargo_test]

0 commit comments

Comments
 (0)