From 8cdc9da2020d4c9e467dd094008dbbb96c87c402 Mon Sep 17 00:00:00 2001 From: AnthonyMikh Date: Sun, 15 Oct 2023 22:48:38 +0400 Subject: [PATCH] appender: remove `Sync` bound from writer for `NonBlocking` (#2607) ## Motivation `NonBlocking` from `tracing-appender` wraps a writer and requires that writer to implement `Sync` (among other bounds). However it is not clear why this bound is necessary in first place: this writer is sent to a dedicated thread and never used directly concurrently. #1934 demonstrates that it is a real problem for some people. Also at my current work we hit this issue as well when a third-party writer did not implement `Sync`. Our workaround was to wrap that writer in our own type and manually implement `Send` and `Sync` for it. Needless to say that it is more a hack than a proper solution. ## Solution Remove `Sync` bound in relevant places. Yep, that simple. Probably having it in first place was just an oversight. Closes #1934 --- tracing-appender/src/lib.rs | 2 +- tracing-appender/src/non_blocking.rs | 6 +++--- tracing-appender/src/worker.rs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tracing-appender/src/lib.rs b/tracing-appender/src/lib.rs index 9fe789e50a..4964e89ac9 100644 --- a/tracing-appender/src/lib.rs +++ b/tracing-appender/src/lib.rs @@ -190,7 +190,7 @@ pub(crate) mod sync; /// }); /// # } /// ``` -pub fn non_blocking(writer: T) -> (NonBlocking, WorkerGuard) { +pub fn non_blocking(writer: T) -> (NonBlocking, WorkerGuard) { NonBlocking::new(writer) } diff --git a/tracing-appender/src/non_blocking.rs b/tracing-appender/src/non_blocking.rs index 1ac2ba54a1..fe43ecc0af 100644 --- a/tracing-appender/src/non_blocking.rs +++ b/tracing-appender/src/non_blocking.rs @@ -146,11 +146,11 @@ impl NonBlocking { /// /// [default]: NonBlockingBuilder::default /// [builder]: NonBlockingBuilder - pub fn new(writer: T) -> (NonBlocking, WorkerGuard) { + pub fn new(writer: T) -> (NonBlocking, WorkerGuard) { NonBlockingBuilder::default().finish(writer) } - fn create( + fn create( writer: T, buffered_lines_limit: usize, is_lossy: bool, @@ -221,7 +221,7 @@ impl NonBlockingBuilder { } /// Completes the builder, returning the configured `NonBlocking`. - pub fn finish(self, writer: T) -> (NonBlocking, WorkerGuard) { + pub fn finish(self, writer: T) -> (NonBlocking, WorkerGuard) { NonBlocking::create( writer, self.buffered_lines_limit, diff --git a/tracing-appender/src/worker.rs b/tracing-appender/src/worker.rs index e913183edd..00a7d38d3d 100644 --- a/tracing-appender/src/worker.rs +++ b/tracing-appender/src/worker.rs @@ -4,7 +4,7 @@ use std::fmt::Debug; use std::io::Write; use std::{io, thread}; -pub(crate) struct Worker { +pub(crate) struct Worker { writer: T, receiver: Receiver, shutdown: Receiver<()>, @@ -18,7 +18,7 @@ pub(crate) enum WorkerState { Shutdown, } -impl Worker { +impl Worker { pub(crate) fn new(receiver: Receiver, writer: T, shutdown: Receiver<()>) -> Worker { Self { writer,