Skip to content

Commit

Permalink
core: lock stdout before printing an error message to stderr
Browse files Browse the repository at this point in the history
Adds a new eprintln_locked macro which locks STDOUT before logging
to STDERR. This patch also replaces instances of eprintln with
eprintln_locked to avoid interleaving lines.

Fixes #1941, Closes #1968
  • Loading branch information
film42 authored and BurntSushi committed Jul 8, 2023
1 parent 4993d29 commit 4782ebd
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 7 deletions.
8 changes: 4 additions & 4 deletions crates/core/logger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl Log for Logger {
fn log(&self, record: &log::Record<'_>) {
match (record.file(), record.line()) {
(Some(file), Some(line)) => {
eprintln!(
eprintln_locked!(
"{}|{}|{}:{}: {}",
record.level(),
record.target(),
Expand All @@ -43,7 +43,7 @@ impl Log for Logger {
);
}
(Some(file), None) => {
eprintln!(
eprintln_locked!(
"{}|{}|{}: {}",
record.level(),
record.target(),
Expand All @@ -52,7 +52,7 @@ impl Log for Logger {
);
}
_ => {
eprintln!(
eprintln_locked!(
"{}|{}: {}",
record.level(),
record.target(),
Expand All @@ -63,6 +63,6 @@ impl Log for Logger {
}

fn flush(&self) {
// We use eprintln! which is flushed on every call.
// We use eprintln_locked! which is flushed on every call.
}
}
2 changes: 1 addition & 1 deletion crates/core/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ type Result<T> = ::std::result::Result<T, Box<dyn error::Error>>;

fn main() {
if let Err(err) = Args::parse().and_then(try_main) {
eprintln!("{}", err);
eprintln_locked!("{}", err);
process::exit(2);
}
}
Expand Down
20 changes: 18 additions & 2 deletions crates/core/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,28 @@ static MESSAGES: AtomicBool = AtomicBool::new(false);
static IGNORE_MESSAGES: AtomicBool = AtomicBool::new(false);
static ERRORED: AtomicBool = AtomicBool::new(false);

/// Like eprintln, but locks STDOUT to prevent interleaving lines.
#[macro_export]
macro_rules! eprintln_locked {
($($tt:tt)*) => {{
{
// This is a bit of an abstraction violation because we explicitly
// lock STDOUT before printing to STDERR. This avoids interleaving
// lines within ripgrep because `search_parallel` uses `termcolor`,
// which accesses the same STDOUT lock when writing lines.
let stdout = std::io::stdout();
let _handle = stdout.lock();
eprintln!($($tt)*);
}
}}
}

/// Emit a non-fatal error message, unless messages were disabled.
#[macro_export]
macro_rules! message {
($($tt:tt)*) => {
if crate::messages::messages() {
eprintln!($($tt)*);
eprintln_locked!($($tt)*);
}
}
}
Expand All @@ -30,7 +46,7 @@ macro_rules! err_message {
macro_rules! ignore_message {
($($tt:tt)*) => {
if crate::messages::messages() && crate::messages::ignore_messages() {
eprintln!($($tt)*);
eprintln_locked!($($tt)*);
}
}
}
Expand Down

0 comments on commit 4782ebd

Please sign in to comment.