Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Buffer unix and lock windows to prevent message interleaving #35975

Merged
merged 1 commit into from
Aug 26, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 85 additions & 2 deletions src/librustc_errors/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,10 @@ impl EmitterWriter {
}
match write!(&mut self.dst, "\n") {
Err(e) => panic!("failed to emit error: {}", e),
_ => ()
_ => match self.dst.flush() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this flush necessary? There's a flush below as well it looks lke.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This flush allows us to print the last "\n" at the end of the errors printing:

error[E0101]: cannot determine a type for this expression: unconstrained type
  --> src/test/compile-fail/E0101.rs:12:13
   |
12 |     let x = |_| {};
   |             ^^^^^^ cannot resolve type of expression

error: aborting due to previous error
jturner-23759: rust jturner$

vs

error[E0101]: cannot determine a type for this expression: unconstrained type
  --> src/test/compile-fail/E0101.rs:12:13
   |
12 |     let x = |_| {};
   |             ^^^^^^ cannot resolve type of expression

error: aborting due to previous error

jturner-23759: rust jturner$

Err(e) => panic!("failed to emit error: {}", e),
_ => ()
}
}
}
}
Expand All @@ -749,6 +752,21 @@ fn overlaps(a1: &Annotation, a2: &Annotation) -> bool {
fn emit_to_destination(rendered_buffer: &Vec<Vec<StyledString>>,
lvl: &Level,
dst: &mut Destination) -> io::Result<()> {
use lock;

// In order to prevent error message interleaving, where multiple error lines get intermixed
// when multiple compiler processes error simultaneously, we emit errors with additional
// steps.
//
// On Unix systems, we write into a buffered terminal rather than directly to a terminal. When
// the .flush() is called we take the buffer created from the buffered writes and write it at
// one shot. Because the Unix systems use ANSI for the colors, which is a text-based styling
// scheme, this buffered approach works and maintains the styling.
//
// On Windows, styling happens through calls to a terminal API. This prevents us from using the
// same buffering approach. Instead, we use a global Windows mutex, which we acquire long
// enough to output the full error message, then we release.
let _buffer_lock = lock::acquire_global_lock("rustc_errors");
for line in rendered_buffer {
for part in line {
dst.apply_style(lvl.clone(), part.style)?;
Expand All @@ -757,6 +775,7 @@ fn emit_to_destination(rendered_buffer: &Vec<Vec<StyledString>>,
}
write!(dst, "\n")?;
}
dst.flush()?;
Ok(())
}

Expand All @@ -783,14 +802,74 @@ fn stderr_isatty() -> bool {
}
}

pub type BufferedStderr = term::Terminal<Output = BufferedWriter> + Send;

pub enum Destination {
Terminal(Box<term::StderrTerminal>),
BufferedTerminal(Box<BufferedStderr>),
Raw(Box<Write + Send>),
}

/// Buffered writer gives us a way on Unix to buffer up an entire error message before we output
/// it. This helps to prevent interleaving of multiple error messages when multiple compiler
/// processes error simultaneously
pub struct BufferedWriter {
buffer: Vec<u8>,
}

impl BufferedWriter {
// note: we use _new because the conditional compilation at its use site may make this
// this function unused on some platforms
fn _new() -> BufferedWriter {
BufferedWriter {
buffer: vec![]
}
}
}

impl Write for BufferedWriter {
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
for b in buf {
self.buffer.push(*b);
}
Ok(buf.len())
}
fn flush(&mut self) -> io::Result<()> {
let mut stderr = io::stderr();
let result = (|| {
stderr.write_all(&self.buffer)?;
stderr.flush()
})();
self.buffer.clear();
result
}
}

impl Destination {
#[cfg(not(windows))]
/// When not on Windows, prefer the buffered terminal so that we can buffer an entire error
/// to be emitted at one time.
fn from_stderr() -> Destination {
let stderr: Option<Box<BufferedStderr>> =
term::TerminfoTerminal::new(BufferedWriter::_new())
.map(|t| Box::new(t) as Box<BufferedStderr>);

match stderr {
Some(t) => BufferedTerminal(t),
None => Raw(Box::new(io::stderr())),
}
}

#[cfg(windows)]
/// Return a normal, unbuffered terminal when on Windows.
fn from_stderr() -> Destination {
match term::stderr() {
let stderr: Option<Box<term::StderrTerminal>> =
term::TerminfoTerminal::new(io::stderr())
.map(|t| Box::new(t) as Box<term::StderrTerminal>)
.or_else(|| term::WinConsole::new(io::stderr()).ok()
.map(|t| Box::new(t) as Box<term::StderrTerminal>));

match stderr {
Some(t) => Terminal(t),
None => Raw(Box::new(io::stderr())),
}
Expand Down Expand Up @@ -839,6 +918,7 @@ impl Destination {
fn start_attr(&mut self, attr: term::Attr) -> io::Result<()> {
match *self {
Terminal(ref mut t) => { t.attr(attr)?; }
BufferedTerminal(ref mut t) => { t.attr(attr)?; }
Raw(_) => { }
}
Ok(())
Expand All @@ -847,6 +927,7 @@ impl Destination {
fn reset_attrs(&mut self) -> io::Result<()> {
match *self {
Terminal(ref mut t) => { t.reset()?; }
BufferedTerminal(ref mut t) => { t.reset()?; }
Raw(_) => { }
}
Ok(())
Expand All @@ -857,12 +938,14 @@ impl Write for Destination {
fn write(&mut self, bytes: &[u8]) -> io::Result<usize> {
match *self {
Terminal(ref mut t) => t.write(bytes),
BufferedTerminal(ref mut t) => t.write(bytes),
Raw(ref mut w) => w.write(bytes),
}
}
fn flush(&mut self) -> io::Result<()> {
match *self {
Terminal(ref mut t) => t.flush(),
BufferedTerminal(ref mut t) => t.flush(),
Raw(ref mut w) => w.flush(),
}
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_errors/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ pub mod emitter;
pub mod snippet;
pub mod registry;
pub mod styled_buffer;
mod lock;

use syntax_pos::{BytePos, Loc, FileLinesResult, FileName, MultiSpan, Span, NO_EXPANSION };
use syntax_pos::{MacroBacktrace};
Expand Down
112 changes: 112 additions & 0 deletions src/librustc_errors/lock.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

//! Bindings to acquire a global named lock.
//!
//! This is intended to be used to synchronize multiple compiler processes to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment should probably be updated.

Copy link
Contributor Author

@sophiajt sophiajt Aug 25, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are you thinking it should be updated?

Update: helps if I actually keep reading past your comment ;) Yes, I'll update that.

//! ensure that we can output complete errors without interleaving on Windows.
//! Note that this is currently only needed for allowing only one 32-bit MSVC
//! linker to execute at once on MSVC hosts, so this is only implemented for
//! `cfg(windows)`. Also note that this may not always be used on Windows,
//! only when targeting 32-bit MSVC.
//!
//! For more information about why this is necessary, see where this is called.

use std::any::Any;

#[cfg(windows)]
#[allow(bad_style)]
pub fn acquire_global_lock(name: &str) -> Box<Any> {
use std::ffi::CString;
use std::io;

type LPSECURITY_ATTRIBUTES = *mut u8;
type BOOL = i32;
type LPCSTR = *const u8;
type HANDLE = *mut u8;
type DWORD = u32;

const INFINITE: DWORD = !0;
const WAIT_OBJECT_0: DWORD = 0;
const WAIT_ABANDONED: DWORD = 0x00000080;

extern "system" {
fn CreateMutexA(lpMutexAttributes: LPSECURITY_ATTRIBUTES,
bInitialOwner: BOOL,
lpName: LPCSTR) -> HANDLE;
fn WaitForSingleObject(hHandle: HANDLE,
dwMilliseconds: DWORD) -> DWORD;
fn ReleaseMutex(hMutex: HANDLE) -> BOOL;
fn CloseHandle(hObject: HANDLE) -> BOOL;
}

struct Handle(HANDLE);

impl Drop for Handle {
fn drop(&mut self) {
unsafe {
CloseHandle(self.0);
}
}
}

struct Guard(Handle);

impl Drop for Guard {
fn drop(&mut self) {
unsafe {
ReleaseMutex((self.0).0);
}
}
}

let cname = CString::new(name).unwrap();
unsafe {
// Create a named mutex, with no security attributes and also not
// acquired when we create it.
//
// This will silently create one if it doesn't already exist, or it'll
// open up a handle to one if it already exists.
let mutex = CreateMutexA(0 as *mut _, 0, cname.as_ptr() as *const u8);
if mutex.is_null() {
panic!("failed to create global mutex named `{}`: {}", name,
io::Error::last_os_error());
}
let mutex = Handle(mutex);

// Acquire the lock through `WaitForSingleObject`.
//
// A return value of `WAIT_OBJECT_0` means we successfully acquired it.
//
// A return value of `WAIT_ABANDONED` means that the previous holder of
// the thread exited without calling `ReleaseMutex`. This can happen,
// for example, when the compiler crashes or is interrupted via ctrl-c
// or the like. In this case, however, we are still transferred
// ownership of the lock so we continue.
//
// If an error happens.. well... that's surprising!
match WaitForSingleObject(mutex.0, INFINITE) {
WAIT_OBJECT_0 | WAIT_ABANDONED => {}
code => {
panic!("WaitForSingleObject failed on global mutex named \
`{}`: {} (ret={:x})", name,
io::Error::last_os_error(), code);
}
}

// Return a guard which will call `ReleaseMutex` when dropped.
Box::new(Guard(mutex))
}
}

#[cfg(unix)]
pub fn acquire_global_lock(_name: &str) -> Box<Any> {
Box::new(())
}