Skip to content

Commit d00c245

Browse files
authored
Auto merge of #35975 - jonathandturner:error_buffering, r=alexcrichton
Buffer unix and lock windows to prevent message interleaving When cargo does a build on multiple processes, multiple crates may error at the same time. If this happens, currently you'll see interleaving of the error messages, which makes for an unreadable message. Example: ``` --> --> src/bin/multithread-unix.rs:16:35src/bin/singlethread.rs:16:24 || 1616 | | Server::new(&addr).workers(8). Server::new(&addr).serveserve(|r: Request| {(|r: Request| { | | ^^^^^^^^^^ expected struct `std::io::Error`, found () expected struct `std::io::Error`, found () || = = notenote: expected type `std::io::Error`: expected type `std::io::Error` = = notenote: found type `()`: found type `()` = = notenote: required because of the requirements on the impl of `futures_minihttp::Service<futures_minihttp::Request, futures_minihttp::Response>` for `[closure@src/bin/multithread-unix.rs:16:41: 22:6]`: required because of the requirements on the impl of `futures_minihttp::Service<futures_minihttp::Request, futures_minihttp::Response>` for `[closure@src/bin/singlethread.rs:16:30: 22:6]` error: aborting due to previous error error: aborting due to previous error ``` This patch uses two techniques to prevent this interleaving. On Unix systems, since they use the text-based ANSI protocol for coloring, we can safely buffer up whole messages before we emit. This PR does this buffering, and emits the whole message at once. On Windows, term must use the Windows terminal API to color the output. This disallows us from using the same buffering technique. Instead, here we grab a Windows mutex (thanks to @alexcrichton for the lock code). This lock only works in Windows and will hold a mutex for the duration of a message output. r? @nikomatsakis
2 parents eaf71f8 + a65b201 commit d00c245

File tree

3 files changed

+198
-2
lines changed

3 files changed

+198
-2
lines changed

src/librustc_errors/emitter.rs

+85-2
Original file line numberDiff line numberDiff line change
@@ -724,7 +724,10 @@ impl EmitterWriter {
724724
}
725725
match write!(&mut self.dst, "\n") {
726726
Err(e) => panic!("failed to emit error: {}", e),
727-
_ => ()
727+
_ => match self.dst.flush() {
728+
Err(e) => panic!("failed to emit error: {}", e),
729+
_ => ()
730+
}
728731
}
729732
}
730733
}
@@ -749,6 +752,21 @@ fn overlaps(a1: &Annotation, a2: &Annotation) -> bool {
749752
fn emit_to_destination(rendered_buffer: &Vec<Vec<StyledString>>,
750753
lvl: &Level,
751754
dst: &mut Destination) -> io::Result<()> {
755+
use lock;
756+
757+
// In order to prevent error message interleaving, where multiple error lines get intermixed
758+
// when multiple compiler processes error simultaneously, we emit errors with additional
759+
// steps.
760+
//
761+
// On Unix systems, we write into a buffered terminal rather than directly to a terminal. When
762+
// the .flush() is called we take the buffer created from the buffered writes and write it at
763+
// one shot. Because the Unix systems use ANSI for the colors, which is a text-based styling
764+
// scheme, this buffered approach works and maintains the styling.
765+
//
766+
// On Windows, styling happens through calls to a terminal API. This prevents us from using the
767+
// same buffering approach. Instead, we use a global Windows mutex, which we acquire long
768+
// enough to output the full error message, then we release.
769+
let _buffer_lock = lock::acquire_global_lock("rustc_errors");
752770
for line in rendered_buffer {
753771
for part in line {
754772
dst.apply_style(lvl.clone(), part.style)?;
@@ -757,6 +775,7 @@ fn emit_to_destination(rendered_buffer: &Vec<Vec<StyledString>>,
757775
}
758776
write!(dst, "\n")?;
759777
}
778+
dst.flush()?;
760779
Ok(())
761780
}
762781

@@ -783,14 +802,74 @@ fn stderr_isatty() -> bool {
783802
}
784803
}
785804

805+
pub type BufferedStderr = term::Terminal<Output = BufferedWriter> + Send;
806+
786807
pub enum Destination {
787808
Terminal(Box<term::StderrTerminal>),
809+
BufferedTerminal(Box<BufferedStderr>),
788810
Raw(Box<Write + Send>),
789811
}
790812

813+
/// Buffered writer gives us a way on Unix to buffer up an entire error message before we output
814+
/// it. This helps to prevent interleaving of multiple error messages when multiple compiler
815+
/// processes error simultaneously
816+
pub struct BufferedWriter {
817+
buffer: Vec<u8>,
818+
}
819+
820+
impl BufferedWriter {
821+
// note: we use _new because the conditional compilation at its use site may make this
822+
// this function unused on some platforms
823+
fn _new() -> BufferedWriter {
824+
BufferedWriter {
825+
buffer: vec![]
826+
}
827+
}
828+
}
829+
830+
impl Write for BufferedWriter {
831+
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
832+
for b in buf {
833+
self.buffer.push(*b);
834+
}
835+
Ok(buf.len())
836+
}
837+
fn flush(&mut self) -> io::Result<()> {
838+
let mut stderr = io::stderr();
839+
let result = (|| {
840+
stderr.write_all(&self.buffer)?;
841+
stderr.flush()
842+
})();
843+
self.buffer.clear();
844+
result
845+
}
846+
}
847+
791848
impl Destination {
849+
#[cfg(not(windows))]
850+
/// When not on Windows, prefer the buffered terminal so that we can buffer an entire error
851+
/// to be emitted at one time.
852+
fn from_stderr() -> Destination {
853+
let stderr: Option<Box<BufferedStderr>> =
854+
term::TerminfoTerminal::new(BufferedWriter::_new())
855+
.map(|t| Box::new(t) as Box<BufferedStderr>);
856+
857+
match stderr {
858+
Some(t) => BufferedTerminal(t),
859+
None => Raw(Box::new(io::stderr())),
860+
}
861+
}
862+
863+
#[cfg(windows)]
864+
/// Return a normal, unbuffered terminal when on Windows.
792865
fn from_stderr() -> Destination {
793-
match term::stderr() {
866+
let stderr: Option<Box<term::StderrTerminal>> =
867+
term::TerminfoTerminal::new(io::stderr())
868+
.map(|t| Box::new(t) as Box<term::StderrTerminal>)
869+
.or_else(|| term::WinConsole::new(io::stderr()).ok()
870+
.map(|t| Box::new(t) as Box<term::StderrTerminal>));
871+
872+
match stderr {
794873
Some(t) => Terminal(t),
795874
None => Raw(Box::new(io::stderr())),
796875
}
@@ -839,6 +918,7 @@ impl Destination {
839918
fn start_attr(&mut self, attr: term::Attr) -> io::Result<()> {
840919
match *self {
841920
Terminal(ref mut t) => { t.attr(attr)?; }
921+
BufferedTerminal(ref mut t) => { t.attr(attr)?; }
842922
Raw(_) => { }
843923
}
844924
Ok(())
@@ -847,6 +927,7 @@ impl Destination {
847927
fn reset_attrs(&mut self) -> io::Result<()> {
848928
match *self {
849929
Terminal(ref mut t) => { t.reset()?; }
930+
BufferedTerminal(ref mut t) => { t.reset()?; }
850931
Raw(_) => { }
851932
}
852933
Ok(())
@@ -857,12 +938,14 @@ impl Write for Destination {
857938
fn write(&mut self, bytes: &[u8]) -> io::Result<usize> {
858939
match *self {
859940
Terminal(ref mut t) => t.write(bytes),
941+
BufferedTerminal(ref mut t) => t.write(bytes),
860942
Raw(ref mut w) => w.write(bytes),
861943
}
862944
}
863945
fn flush(&mut self) -> io::Result<()> {
864946
match *self {
865947
Terminal(ref mut t) => t.flush(),
948+
BufferedTerminal(ref mut t) => t.flush(),
866949
Raw(ref mut w) => w.flush(),
867950
}
868951
}

src/librustc_errors/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ pub mod emitter;
5050
pub mod snippet;
5151
pub mod registry;
5252
pub mod styled_buffer;
53+
mod lock;
5354

5455
use syntax_pos::{BytePos, Loc, FileLinesResult, FileName, MultiSpan, Span, NO_EXPANSION };
5556
use syntax_pos::{MacroBacktrace};

src/librustc_errors/lock.rs

+112
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
//! Bindings to acquire a global named lock.
12+
//!
13+
//! This is intended to be used to synchronize multiple compiler processes to
14+
//! ensure that we can output complete errors without interleaving on Windows.
15+
//! Note that this is currently only needed for allowing only one 32-bit MSVC
16+
//! linker to execute at once on MSVC hosts, so this is only implemented for
17+
//! `cfg(windows)`. Also note that this may not always be used on Windows,
18+
//! only when targeting 32-bit MSVC.
19+
//!
20+
//! For more information about why this is necessary, see where this is called.
21+
22+
use std::any::Any;
23+
24+
#[cfg(windows)]
25+
#[allow(bad_style)]
26+
pub fn acquire_global_lock(name: &str) -> Box<Any> {
27+
use std::ffi::CString;
28+
use std::io;
29+
30+
type LPSECURITY_ATTRIBUTES = *mut u8;
31+
type BOOL = i32;
32+
type LPCSTR = *const u8;
33+
type HANDLE = *mut u8;
34+
type DWORD = u32;
35+
36+
const INFINITE: DWORD = !0;
37+
const WAIT_OBJECT_0: DWORD = 0;
38+
const WAIT_ABANDONED: DWORD = 0x00000080;
39+
40+
extern "system" {
41+
fn CreateMutexA(lpMutexAttributes: LPSECURITY_ATTRIBUTES,
42+
bInitialOwner: BOOL,
43+
lpName: LPCSTR) -> HANDLE;
44+
fn WaitForSingleObject(hHandle: HANDLE,
45+
dwMilliseconds: DWORD) -> DWORD;
46+
fn ReleaseMutex(hMutex: HANDLE) -> BOOL;
47+
fn CloseHandle(hObject: HANDLE) -> BOOL;
48+
}
49+
50+
struct Handle(HANDLE);
51+
52+
impl Drop for Handle {
53+
fn drop(&mut self) {
54+
unsafe {
55+
CloseHandle(self.0);
56+
}
57+
}
58+
}
59+
60+
struct Guard(Handle);
61+
62+
impl Drop for Guard {
63+
fn drop(&mut self) {
64+
unsafe {
65+
ReleaseMutex((self.0).0);
66+
}
67+
}
68+
}
69+
70+
let cname = CString::new(name).unwrap();
71+
unsafe {
72+
// Create a named mutex, with no security attributes and also not
73+
// acquired when we create it.
74+
//
75+
// This will silently create one if it doesn't already exist, or it'll
76+
// open up a handle to one if it already exists.
77+
let mutex = CreateMutexA(0 as *mut _, 0, cname.as_ptr() as *const u8);
78+
if mutex.is_null() {
79+
panic!("failed to create global mutex named `{}`: {}", name,
80+
io::Error::last_os_error());
81+
}
82+
let mutex = Handle(mutex);
83+
84+
// Acquire the lock through `WaitForSingleObject`.
85+
//
86+
// A return value of `WAIT_OBJECT_0` means we successfully acquired it.
87+
//
88+
// A return value of `WAIT_ABANDONED` means that the previous holder of
89+
// the thread exited without calling `ReleaseMutex`. This can happen,
90+
// for example, when the compiler crashes or is interrupted via ctrl-c
91+
// or the like. In this case, however, we are still transferred
92+
// ownership of the lock so we continue.
93+
//
94+
// If an error happens.. well... that's surprising!
95+
match WaitForSingleObject(mutex.0, INFINITE) {
96+
WAIT_OBJECT_0 | WAIT_ABANDONED => {}
97+
code => {
98+
panic!("WaitForSingleObject failed on global mutex named \
99+
`{}`: {} (ret={:x})", name,
100+
io::Error::last_os_error(), code);
101+
}
102+
}
103+
104+
// Return a guard which will call `ReleaseMutex` when dropped.
105+
Box::new(Guard(mutex))
106+
}
107+
}
108+
109+
#[cfg(unix)]
110+
pub fn acquire_global_lock(_name: &str) -> Box<Any> {
111+
Box::new(())
112+
}

0 commit comments

Comments
 (0)