fix: stop duplicating every log line to the file
When log buffering and a LOG_FILE were both active, log() wrote each line to the file immediately and also pushed it to the buffer, which flush_buffered_logs then wrote to the file a second time. Each line now routes to exactly one sink (file / buffer / stderr) via a small tested decision function; a configured file always bypasses buffering, so nothing is written twice.
This commit is contained in:
@@ -64,6 +64,31 @@ pub fn init() {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Where a single log line should be written. Each line goes to exactly one
|
||||||
|
/// sink; routing a line to more than one is what caused file lines to be
|
||||||
|
/// duplicated (written once eagerly and again when the buffer was flushed).
|
||||||
|
#[derive(Debug, PartialEq, Eq)]
|
||||||
|
enum LogSink {
|
||||||
|
File,
|
||||||
|
Buffer,
|
||||||
|
Stderr,
|
||||||
|
}
|
||||||
|
|
||||||
|
fn choose_sink(buffering: bool, has_file: bool) -> LogSink {
|
||||||
|
match (buffering, has_file) {
|
||||||
|
// A configured file always takes the line directly and bypasses
|
||||||
|
// buffering, so the flush step never re-writes it.
|
||||||
|
(_, true) => LogSink::File,
|
||||||
|
// No file: while the TUI holds the screen we defer stderr writes.
|
||||||
|
(true, false) => LogSink::Buffer,
|
||||||
|
(false, false) => LogSink::Stderr,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn has_log_file() -> bool {
|
||||||
|
LOG_FILE.lock().map(|f| f.is_some()).unwrap_or(false)
|
||||||
|
}
|
||||||
|
|
||||||
pub fn log(level: LogLevel, message: &str) {
|
pub fn log(level: LogLevel, message: &str) {
|
||||||
unsafe {
|
unsafe {
|
||||||
if level <= LOG_LEVEL {
|
if level <= LOG_LEVEL {
|
||||||
@@ -161,29 +186,23 @@ pub fn log(level: LogLevel, message: &str) {
|
|||||||
message
|
message
|
||||||
);
|
);
|
||||||
|
|
||||||
if BUFFERING.load(Ordering::Relaxed) {
|
match choose_sink(BUFFERING.load(Ordering::Relaxed), has_log_file())
|
||||||
// When buffering is enabled, add to buffer for stderr
|
{
|
||||||
if let Ok(mut buffer) = BUFFER.lock() {
|
LogSink::File => {
|
||||||
buffer.push(formatted_message.clone());
|
if let Ok(mut log_file) = LOG_FILE.lock()
|
||||||
}
|
&& let Some(file) = log_file.as_mut()
|
||||||
// But ALSO write to file if configured (file logging bypasses buffering)
|
{
|
||||||
if let Ok(mut log_file) = LOG_FILE.lock()
|
let _ =
|
||||||
&& let Some(file) = log_file.as_mut()
|
writeln!(file, "{}", formatted_message_plain);
|
||||||
{
|
|
||||||
let _ = writeln!(file, "{}", formatted_message_plain);
|
|
||||||
let _ = file.flush();
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
// Write to file if configured, otherwise to stderr
|
|
||||||
if let Ok(mut log_file) = LOG_FILE.lock() {
|
|
||||||
if let Some(file) = log_file.as_mut() {
|
|
||||||
let _ = writeln!(file, "{}", formatted_message_plain);
|
|
||||||
let _ = file.flush();
|
let _ = file.flush();
|
||||||
} else {
|
|
||||||
eprintln!("{}", formatted_message);
|
|
||||||
}
|
}
|
||||||
} else {
|
}
|
||||||
// If lock fails, fall back to stderr
|
LogSink::Buffer => {
|
||||||
|
if let Ok(mut buffer) = BUFFER.lock() {
|
||||||
|
buffer.push(formatted_message.clone());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
LogSink::Stderr => {
|
||||||
eprintln!("{}", formatted_message);
|
eprintln!("{}", formatted_message);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -251,6 +270,26 @@ fn strip_ansi_codes(s: &str) -> String {
|
|||||||
result
|
result
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::{LogSink, choose_sink};
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_configured_file_bypasses_buffering_so_lines_are_not_duplicated() {
|
||||||
|
// With a file configured, the line goes straight to the file whether or
|
||||||
|
// not buffering is on, so it is never also queued for a flush that would
|
||||||
|
// write it a second time.
|
||||||
|
assert_eq!(choose_sink(true, true), LogSink::File);
|
||||||
|
assert_eq!(choose_sink(false, true), LogSink::File);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn without_a_file_buffering_routes_to_the_buffer_else_stderr() {
|
||||||
|
assert_eq!(choose_sink(true, false), LogSink::Buffer);
|
||||||
|
assert_eq!(choose_sink(false, false), LogSink::Stderr);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[macro_export]
|
#[macro_export]
|
||||||
macro_rules! error {
|
macro_rules! error {
|
||||||
($($arg:tt)*) => {
|
($($arg:tt)*) => {
|
||||||
|
|||||||
Reference in New Issue
Block a user