Skip to content

Commit f461c5b

Browse files
hawkwdzvon
andcommitted
appender: fix compilation 32-bit platforms like PowerPC (#1675)
This branch is @dzvon's PR #1508, with the following changes: * Add a newtype wrapping the error counter to hide that it's internally an `Arc<AtomicUsize>`. This would allow us to make additional changes to the implementation without potentially causing breaking changes. * Use saturating arithmetic when incrementing the counter to avoid wrapping to 0 on overflows. This is more likely to be an issue on 32-bit platforms. This is a breaking change that will be released as part of `tracing-appender` 0.2. Closes #1508 Description from @dzvon's original PR: ## Motivation Currently, tracing-appender crate cannot be compiled on PowerPC platform. Because > PowerPC and MIPS platforms with 32-bit pointers do not have > `AtomicU64` or `AtomicI64` types. quote from std library docs. (https://doc.rust-lang.org/std/sync/atomic/index.html#portability) ## Solution Change `AtomicU64` to `AtomicUsize`. Co-authored-by: Dezhi Wu <[email protected]>
1 parent 8777d2c commit f461c5b

File tree

1 file changed

+59
-13
lines changed

1 file changed

+59
-13
lines changed

Diff for: tracing-appender/src/non_blocking.rs

+59-13
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ use crate::Msg;
5151
use crossbeam_channel::{bounded, SendTimeoutError, Sender};
5252
use std::io;
5353
use std::io::Write;
54-
use std::sync::atomic::AtomicU64;
54+
use std::sync::atomic::AtomicUsize;
5555
use std::sync::atomic::Ordering;
5656
use std::sync::Arc;
5757
use std::thread::JoinHandle;
@@ -125,11 +125,20 @@ pub struct WorkerGuard {
125125
/// [fmt]: https://docs.rs/tracing-subscriber/latest/tracing_subscriber/fmt/index.html
126126
#[derive(Clone, Debug)]
127127
pub struct NonBlocking {
128-
error_counter: Arc<AtomicU64>,
128+
error_counter: ErrorCounter,
129129
channel: Sender<Msg>,
130130
is_lossy: bool,
131131
}
132132

133+
/// Tracks the number of times a log line was dropped by the background thread.
134+
///
135+
/// If the non-blocking writer is not configured in [lossy mode], the error
136+
/// count should always be 0.
137+
///
138+
/// [lossy mode]: NonBlockingBuilder::lossy
139+
#[derive(Clone, Debug)]
140+
pub struct ErrorCounter(Arc<AtomicUsize>);
141+
133142
impl NonBlocking {
134143
/// Returns a new `NonBlocking` writer wrapping the provided `writer`.
135144
///
@@ -158,7 +167,7 @@ impl NonBlocking {
158167
(
159168
Self {
160169
channel: sender,
161-
error_counter: Arc::new(AtomicU64::new(0)),
170+
error_counter: ErrorCounter(Arc::new(AtomicUsize::new(0))),
162171
is_lossy,
163172
},
164173
worker_guard,
@@ -167,7 +176,7 @@ impl NonBlocking {
167176

168177
/// Returns a counter for the number of times logs where dropped. This will always return zero if
169178
/// `NonBlocking` is not lossy.
170-
pub fn error_counter(&self) -> Arc<AtomicU64> {
179+
pub fn error_counter(&self) -> ErrorCounter {
171180
self.error_counter.clone()
172181
}
173182
}
@@ -219,7 +228,7 @@ impl std::io::Write for NonBlocking {
219228
let buf_size = buf.len();
220229
if self.is_lossy {
221230
if self.channel.try_send(Msg::Line(buf.to_vec())).is_err() {
222-
self.error_counter.fetch_add(1, Ordering::Release);
231+
self.error_counter.incr_saturating();
223232
}
224233
} else {
225234
return match self.channel.send(Msg::Line(buf.to_vec())) {
@@ -280,6 +289,43 @@ impl Drop for WorkerGuard {
280289
}
281290
}
282291

292+
// === impl ErrorCounter ===
293+
294+
impl ErrorCounter {
295+
/// Returns the number of log lines that have been dropped.
296+
///
297+
/// If the non-blocking writer is not configured in [lossy mode], the error
298+
/// count should always be 0.
299+
///
300+
/// [lossy mode]: NonBlockingBuilder::lossy
301+
pub fn dropped_lines(&self) -> usize {
302+
self.0.load(Ordering::Acquire)
303+
}
304+
305+
fn incr_saturating(&self) {
306+
let mut curr = self.0.load(Ordering::Acquire);
307+
// We don't need to enter the CAS loop if the current value is already
308+
// `usize::MAX`.
309+
if curr == usize::MAX {
310+
return;
311+
}
312+
313+
// This is implemented as a CAS loop rather than as a simple
314+
// `fetch_add`, because we don't want to wrap on overflow. Instead, we
315+
// need to ensure that saturating addition is performed.
316+
loop {
317+
let val = curr.saturating_add(1);
318+
match self
319+
.0
320+
.compare_exchange(curr, val, Ordering::AcqRel, Ordering::Acquire)
321+
{
322+
Ok(_) => return,
323+
Err(actual) => curr = actual,
324+
}
325+
}
326+
}
327+
}
328+
283329
#[cfg(test)]
284330
mod test {
285331
use super::*;
@@ -322,7 +368,7 @@ mod test {
322368
let error_count = non_blocking.error_counter();
323369

324370
non_blocking.write_all(b"Hello").expect("Failed to write");
325-
assert_eq!(0, error_count.load(Ordering::Acquire));
371+
assert_eq!(0, error_count.dropped_lines());
326372

327373
let handle = thread::spawn(move || {
328374
non_blocking.write_all(b", World").expect("Failed to write");
@@ -331,7 +377,7 @@ mod test {
331377
// Sleep a little to ensure previously spawned thread gets blocked on write.
332378
thread::sleep(Duration::from_millis(100));
333379
// We should not drop logs when blocked.
334-
assert_eq!(0, error_count.load(Ordering::Acquire));
380+
assert_eq!(0, error_count.dropped_lines());
335381

336382
// Read the first message to unblock sender.
337383
let mut line = rx.recv().unwrap();
@@ -366,17 +412,17 @@ mod test {
366412

367413
// First write will not block
368414
write_non_blocking(&mut non_blocking, b"Hello");
369-
assert_eq!(0, error_count.load(Ordering::Acquire));
415+
assert_eq!(0, error_count.dropped_lines());
370416

371417
// Second write will not block as Worker will have called `recv` on channel.
372418
// "Hello" is not yet consumed. MockWriter call to write_all will block until
373419
// "Hello" is consumed.
374420
write_non_blocking(&mut non_blocking, b", World");
375-
assert_eq!(0, error_count.load(Ordering::Acquire));
421+
assert_eq!(0, error_count.dropped_lines());
376422

377423
// Will sit in NonBlocking channel's buffer.
378424
write_non_blocking(&mut non_blocking, b"Test");
379-
assert_eq!(0, error_count.load(Ordering::Acquire));
425+
assert_eq!(0, error_count.dropped_lines());
380426

381427
// Allow a line to be written. "Hello" message will be consumed.
382428
// ", World" will be able to write to MockWriter.
@@ -386,12 +432,12 @@ mod test {
386432

387433
// This will block as NonBlocking channel is full.
388434
write_non_blocking(&mut non_blocking, b"Universe");
389-
assert_eq!(1, error_count.load(Ordering::Acquire));
435+
assert_eq!(1, error_count.dropped_lines());
390436

391437
// Finally the second message sent will be consumed.
392438
let line = rx.recv().unwrap();
393439
assert_eq!(line, ", World");
394-
assert_eq!(1, error_count.load(Ordering::Acquire));
440+
assert_eq!(1, error_count.dropped_lines());
395441
}
396442

397443
#[test]
@@ -427,6 +473,6 @@ mod test {
427473
}
428474

429475
assert_eq!(10, hello_count);
430-
assert_eq!(0, error_count.load(Ordering::Acquire));
476+
assert_eq!(0, error_count.dropped_lines());
431477
}
432478
}

0 commit comments

Comments
 (0)