Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .github/workflows/rust-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,16 @@ jobs:
- run: cargo fetch
- name: cargo publish check
run: cargo publish --dry-run

old-glibc:
name: Test (rockylinux:8, glibc 2.28)
runs-on: ubuntu-24.04
# Ubuntu's glibc exports symbols RHEL 8 lacks (gettid needs 2.30), so a
# green ubuntu run says nothing about whether this links there.
container: rockylinux:8
steps:
- run: dnf install -y gcc
- uses: actions/checkout@v7
- uses: dtolnay/rust-toolchain@stable
- uses: Swatinem/rust-cache@v2
- run: cargo test --all-features
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ tracing-core = "0.1"
tracing-log = { version = "0.2", default-features = false, optional = true }
tracing-subscriber = { version = "0.3", default-features = false, features = ["fmt"] }

[target.'cfg(any(target_os = "linux", target_os = "android", target_vendor = "apple"))'.dependencies]
libc = "0.2"

[dev-dependencies]
tracing-log = "0.2"

Expand Down
68 changes: 6 additions & 62 deletions src/formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use tracing_subscriber::fmt::{FmtContext, FormattedFields};
use tracing_subscriber::registry::LookupSpan;
use tracing_subscriber::Layer;

use crate::os;
use crate::serializer::{Serializer, SerializerError};

/// Creates a formatting layer
Expand Down Expand Up @@ -74,65 +75,6 @@ fn default_enable_ansi_color() -> bool {
std::io::stdout().is_terminal()
}

/// Gets the current thread id as an integer
///
/// This only supports the normal platforms here (Linux, Windows, Apple).
/// For anything else like WASM we just return 0.
///
/// Note that a Rust `ThreadId` is actually just a process specific atomic,
/// completely unrelated to the thread id assigned by the operating system, but
/// we use the operating system one, at least until they stabilize
/// <https://doc.rust-lang.org/std/thread/struct.ThreadId.html#method.as_u64>
#[inline]
#[allow(unsafe_code)]
fn current_thread_id() -> u64 {
#[cfg(any(target_os = "linux", target_os = "android"))]
{
#[link(name = "c")]
unsafe extern "C" {
safe fn gettid() -> i32;
}

gettid() as _
}

#[cfg(target_os = "windows")]
{
#[link(name = "kernel32", kind = "raw-dylib")]
unsafe extern "system" {
safe fn GetCurrentThreadId() -> u32;
}

GetCurrentThreadId() as _
}

#[cfg(target_vendor = "apple")]
{
#[link(name = "c")]
unsafe extern "C" {
// technically isn't safe since it accepts a pointer, but we only
// have a single use that keeps the pointee on the stack
safe fn pthread_threadid_np(thread: usize, id: *mut u64) -> i32;
}

let mut id = 0;
if pthread_threadid_np(0, &mut id) != 0 {
id = 0;
}

id
}

#[cfg(not(any(
any(target_os = "linux", target_os = "android"),
target_os = "windows",
target_vendor = "apple"
)))]
{
0
}
}

impl<S, N> FormatEvent<S, N> for EventsFormatter
where
S: Subscriber + for<'a> LookupSpan<'a>,
Expand Down Expand Up @@ -214,13 +156,15 @@ where
serializer.serialize_entry("thread_name", name)?;
} else if !self.with_thread_ids {
// fall-back to thread id when name is absent and ids are not enabled
serializer
.serialize_entry("thread_name", &format!("{}", current_thread_id()))?;
serializer.serialize_entry(
"thread_name",
&format!("{}", os::gettid().unwrap_or(0)),
)?;
}
}

if self.with_thread_ids {
serializer.serialize_entry_no_quote("thread_id", current_thread_id())?;
serializer.serialize_entry_no_quote("thread_id", os::gettid().unwrap_or(0))?;
}

let span = if self.with_span_name || self.with_span_path {
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
#![deny(unreachable_pub)]

mod formatter;
mod os;
mod serializer;

pub use crate::formatter::builder::{builder, Builder};
Expand Down
14 changes: 14 additions & 0 deletions src/os/apple.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//! Apple-specific OS utilities.
#![allow(unsafe_code, reason = "need to call system utilities in os code")]

/// Get the current system thread id.
#[inline]
pub(super) fn gettid() -> u64 {
let mut id: u64 = 0;
// SAFETY: id is stack-allocated and guaranteed to outlast the call.
let res = unsafe { libc::pthread_threadid_np(0, &mut id) };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 macOS builds fail

libc::pthread_threadid_np takes a pointer-typed pthread_t on Apple, but this call passes the integer 0. Rust does not convert that integer into a null pointer, so macOS builds fail with a type mismatch. Pass std::ptr::null_mut() instead.

Suggested change
let res = unsafe { libc::pthread_threadid_np(0, &mut id) };
let res = unsafe { libc::pthread_threadid_np(std::ptr::null_mut(), &mut id) };

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not true: on Apple, libc defines pthread_t = crate::uintptr_t (src/unix/bsd/mod.rs:8 in libc 0.2.191), so 0 is a usize and Test (macos-14) passes. std::ptr::null_mut() is what would fail to compile.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right—my comment was incorrect. With the locked libc 0.2.191 binding, Apple’s pthread_t is uintptr_t (usize), so 0 is type-correct here, while std::ptr::null_mut() would fail to compile. The macOS CI pass is consistent with that. Please disregard my suggested change; the existing call is correct.


(res == 0)
.then_some(id)
.expect("pthread_threadid_np only fails for NULL id or dead thread")
}
28 changes: 28 additions & 0 deletions src/os/linux/glibc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//! libc's `gettid`, looked up at runtime since glibc < 2.30 does not export it.

type GetTidFn = extern "C" fn() -> libc::pid_t;

/// Looks up libc's `gettid`, if it exports one.
///
/// The lookup runs once per process.
fn gettid_fn() -> Option<GetTidFn> {
static GETTID: std::sync::OnceLock<Option<GetTidFn>> = std::sync::OnceLock::new();

*GETTID.get_or_init(|| {
// SAFETY: the name is NUL-terminated and `RTLD_DEFAULT` is a valid handle.
let res: *mut libc::c_void = unsafe { libc::dlsym(libc::RTLD_DEFAULT, c"gettid".as_ptr()) };

if res.is_null() {
return None;
}

// SAFETY: libc's `gettid` is `pid_t gettid(void)`, which is what
// `GetTidFn` declares, and `res` is not null.
Some(unsafe { std::mem::transmute::<*mut libc::c_void, GetTidFn>(res) })
})
}

/// Gets the calling thread's id from libc's `gettid`, if libc exports one.
pub(super) fn gettid() -> Option<u64> {
gettid_fn().map(|gettid| gettid() as u64)
}
15 changes: 15 additions & 0 deletions src/os/linux/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//! Linux-specific OS utilities.
#![allow(unsafe_code, reason = "need to call system utilities in os code")]

mod glibc;

/// Get the current thread's system ID.
#[inline]
pub(super) fn gettid() -> u64 {
fn syscall_gettid() -> u64 {
// SAFETY: the gettid syscall takes no arguments and touches no memory.
unsafe { libc::syscall(libc::SYS_gettid) as u64 }
}

glibc::gettid().unwrap_or_else(syscall_gettid)
}
66 changes: 66 additions & 0 deletions src/os/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
//! Operating system utilities.

#[cfg(target_vendor = "apple")]
mod apple;
#[cfg(any(target_os = "linux", target_os = "android"))]
mod linux;
#[cfg(target_os = "windows")]
mod windows;

/// Gets the current thread id as an integer.
///
/// [`Option::None`] if the active thread could not be determined or the
/// platform doesn't support it.
///
/// Note that a Rust `ThreadId` is actually just a process specific atomic,
/// completely unrelated to the thread id assigned by the operating system, but
/// we use the operating system one, at least until they stabilize
/// <https://doc.rust-lang.org/std/thread/struct.ThreadId.html#method.as_u64>
#[inline]
pub(crate) fn gettid() -> Option<u64> {
#[cfg(target_vendor = "apple")]
{
Some(apple::gettid())
}
#[cfg(any(target_os = "linux", target_os = "android"))]
{
Some(linux::gettid())
}
#[cfg(target_os = "windows")]
{
Some(windows::gettid())
}

#[cfg(not(any(
target_vendor = "apple",
target_os = "linux",
target_os = "android",
target_os = "windows"
)))]
None
}

#[cfg(all(test, any(target_os = "linux", target_os = "android")))]
mod tests {
use super::*;

#[test]
fn gettid_returns_the_kernel_tid() {
// A new thread, since under `--test-threads=1` this runs on the main
// thread, whose tid is also the pid.
let (got, tid) = std::thread::spawn(|| {
let link = std::fs::read_link("/proc/thread-self").expect("procfs is mounted");
let tid: u64 = link
.file_name()
.and_then(|name| name.to_str())
.and_then(|name| name.parse().ok())
.expect("/proc/thread-self ends in the tid");

(gettid(), tid)
})
.join()
.expect("the probe thread does not panic");

assert_eq!(got, Some(tid));
}
}
13 changes: 13 additions & 0 deletions src/os/windows.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//! Windows-specific OS utilities.
#![allow(unsafe_code, reason = "need to call system utilities in os code")]

#[link(name = "kernel32", kind = "raw-dylib")]
unsafe extern "system" {
safe fn GetCurrentThreadId() -> u32;
}

/// Get the current system thread id.
#[inline]
pub(super) fn gettid() -> u64 {
GetCurrentThreadId() as _
}
Loading