Skip to content

Commit 87496cf

Browse files
authored
Merge pull request #146 from divergentdave/thread-local-fallback
Thread local fallback
2 parents 424f031 + ecfc941 commit 87496cf

File tree

3 files changed

+109
-31
lines changed

3 files changed

+109
-31
lines changed

Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ harness = false
3434
name = "log-in-log"
3535
harness = false
3636

37+
[[test]]
38+
name = "log_tls_dtors"
39+
harness = false
40+
3741
[[test]]
3842
name = "init-twice-retains-filter"
3943
harness = false

src/lib.rs

Lines changed: 39 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -807,41 +807,49 @@ impl Log for Logger {
807807
static FORMATTER: RefCell<Option<Formatter>> = RefCell::new(None);
808808
}
809809

810-
FORMATTER.with(|tl_buf| {
811-
// It's possible for implementations to sometimes
812-
// log-while-logging (e.g. a `std::fmt` implementation logs
813-
// internally) but it's super rare. If this happens make sure we
814-
// at least don't panic and ship some output to the screen.
815-
let mut a;
816-
let mut b = None;
817-
let tl_buf = match tl_buf.try_borrow_mut() {
818-
Ok(f) => {
819-
a = f;
820-
&mut *a
821-
}
822-
Err(_) => &mut b,
823-
};
824-
825-
// Check the buffer style. If it's different from the logger's
826-
// style then drop the buffer and recreate it.
827-
match *tl_buf {
828-
Some(ref mut formatter) => {
829-
if formatter.write_style() != self.writer.write_style() {
830-
*formatter = Formatter::new(&self.writer)
831-
}
832-
}
833-
ref mut tl_buf => *tl_buf = Some(Formatter::new(&self.writer)),
834-
}
835-
836-
// The format is guaranteed to be `Some` by this point
837-
let mut formatter = tl_buf.as_mut().unwrap();
838-
839-
let _ = (self.format)(&mut formatter, record)
810+
let print = |formatter: &mut Formatter, record: &Record| {
811+
let _ = (self.format)(formatter, record)
840812
.and_then(|_| formatter.print(&self.writer));
841813

842814
// Always clear the buffer afterwards
843815
formatter.clear();
844-
});
816+
};
817+
818+
let printed = FORMATTER.try_with(|tl_buf| {
819+
match tl_buf.try_borrow_mut() {
820+
// There are no active borrows of the buffer
821+
Ok(mut tl_buf) => match *tl_buf {
822+
// We have a previously set formatter
823+
Some(ref mut formatter) => {
824+
// Check the buffer style. If it's different from the logger's
825+
// style then drop the buffer and recreate it.
826+
if formatter.write_style() != self.writer.write_style() {
827+
*formatter = Formatter::new(&self.writer);
828+
}
829+
830+
print(formatter, record);
831+
}
832+
// We don't have a previously set formatter
833+
None => {
834+
let mut formatter = Formatter::new(&self.writer);
835+
print(&mut formatter, record);
836+
837+
*tl_buf = Some(formatter);
838+
}
839+
}
840+
// There's already an active borrow of the buffer (due to re-entrancy)
841+
Err(_) => {
842+
print(&mut Formatter::new(&self.writer), record);
843+
}
844+
}
845+
}).is_ok();
846+
847+
if !printed {
848+
// The thread-local storage was not available (because its
849+
// destructor has already run). Create a new single-use
850+
// Formatter on the stack for this call.
851+
print(&mut Formatter::new(&self.writer), record);
852+
}
845853
}
846854
}
847855

tests/log_tls_dtors.rs

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
#[macro_use]
2+
extern crate log;
3+
extern crate env_logger;
4+
5+
use std::env;
6+
use std::process;
7+
use std::str;
8+
use std::thread;
9+
10+
struct DropMe;
11+
12+
impl Drop for DropMe {
13+
fn drop(&mut self) {
14+
debug!("Dropping now");
15+
}
16+
}
17+
18+
fn run() {
19+
// Use multiple thread local values to increase the chance that our TLS
20+
// value will get destroyed after the FORMATTER key in the library
21+
thread_local! {
22+
static DROP_ME_0: DropMe = DropMe;
23+
static DROP_ME_1: DropMe = DropMe;
24+
static DROP_ME_2: DropMe = DropMe;
25+
static DROP_ME_3: DropMe = DropMe;
26+
static DROP_ME_4: DropMe = DropMe;
27+
static DROP_ME_5: DropMe = DropMe;
28+
static DROP_ME_6: DropMe = DropMe;
29+
static DROP_ME_7: DropMe = DropMe;
30+
static DROP_ME_8: DropMe = DropMe;
31+
static DROP_ME_9: DropMe = DropMe;
32+
}
33+
DROP_ME_0.with(|_| {});
34+
DROP_ME_1.with(|_| {});
35+
DROP_ME_2.with(|_| {});
36+
DROP_ME_3.with(|_| {});
37+
DROP_ME_4.with(|_| {});
38+
DROP_ME_5.with(|_| {});
39+
DROP_ME_6.with(|_| {});
40+
DROP_ME_7.with(|_| {});
41+
DROP_ME_8.with(|_| {});
42+
DROP_ME_9.with(|_| {});
43+
}
44+
45+
fn main() {
46+
env_logger::init();
47+
if env::var("YOU_ARE_TESTING_NOW").is_ok() {
48+
// Run on a separate thread because TLS values on the main thread
49+
// won't have their destructors run if pthread is used.
50+
// https://doc.rust-lang.org/std/thread/struct.LocalKey.html#platform-specific-behavior
51+
thread::spawn(run).join().unwrap();
52+
} else {
53+
let exe = env::current_exe().unwrap();
54+
let out = process::Command::new(exe)
55+
.env("YOU_ARE_TESTING_NOW", "1")
56+
.env("RUST_LOG", "debug")
57+
.output()
58+
.unwrap_or_else(|e| panic!("Unable to start child process: {}", e));
59+
if !out.status.success() {
60+
println!("test failed: {}", out.status);
61+
println!("--- stdout\n{}", str::from_utf8(&out.stdout).unwrap());
62+
println!("--- stderr\n{}", str::from_utf8(&out.stderr).unwrap());
63+
process::exit(1);
64+
}
65+
}
66+
}

0 commit comments

Comments
 (0)