From 2dfb7c90697af2eea8089046ec899d4c518f2aea Mon Sep 17 00:00:00 2001
From: Garrett Thornburg <film42@gmail.com>
Date: Mon, 9 Aug 2021 23:32:08 -0600
Subject: [PATCH] core: lock stdout before printing an error message to stderr

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
---
 crates/core/logger.rs   |  8 ++++----
 crates/core/main.rs     |  2 +-
 crates/core/messages.rs | 20 ++++++++++++++++++--
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/crates/core/logger.rs b/crates/core/logger.rs
index 0fe063f1ce..0c5414c75f 100644
--- a/crates/core/logger.rs
+++ b/crates/core/logger.rs
@@ -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(),
@@ -43,7 +43,7 @@ impl Log for Logger {
                 );
             }
             (Some(file), None) => {
-                eprintln!(
+                eprintln_locked!(
                     "{}|{}|{}: {}",
                     record.level(),
                     record.target(),
@@ -52,7 +52,7 @@ impl Log for Logger {
                 );
             }
             _ => {
-                eprintln!(
+                eprintln_locked!(
                     "{}|{}: {}",
                     record.level(),
                     record.target(),
@@ -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.
     }
 }
diff --git a/crates/core/main.rs b/crates/core/main.rs
index 2584ea9f14..eff4da1f30 100644
--- a/crates/core/main.rs
+++ b/crates/core/main.rs
@@ -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);
     }
 }
diff --git a/crates/core/messages.rs b/crates/core/messages.rs
index dc013e1ced..be9e10dcab 100644
--- a/crates/core/messages.rs
+++ b/crates/core/messages.rs
@@ -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)*);
         }
     }
 }
@@ -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)*);
         }
     }
 }