From 6906ff6b3598277211f1c5f0201a3a7b5b78d7b5 Mon Sep 17 00:00:00 2001 From: Boxy Date: Thu, 13 Mar 2025 15:49:35 +0000 Subject: [PATCH 1/2] Remove object lifetime cast --- metrics/src/recorder/mod.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/metrics/src/recorder/mod.rs b/metrics/src/recorder/mod.rs index ca251635..b2f61538 100644 --- a/metrics/src/recorder/mod.rs +++ b/metrics/src/recorder/mod.rs @@ -140,10 +140,16 @@ pub struct LocalRecorderGuard<'a> { impl<'a> LocalRecorderGuard<'a> { /// Creates a new `LocalRecorderGuard` and sets the thread-local recorder. fn new(recorder: &'a dyn Recorder) -> Self { + // SAFETY: Why this it okay to extend the lifetime of the object type from `'a` to `'static`? + let recorder_ptr = unsafe { + std::mem::transmute::<*const (dyn Recorder + '_), *mut (dyn Recorder + 'static)>( + recorder as &dyn Recorder, + ) + }; // SAFETY: While we take a lifetime-less pointer to the given reference, the reference we derive _from_ the // pointer is given the same lifetime of the reference used to construct the guard -- captured in the guard type // itself -- and so derived references never outlive the source reference. - let recorder_ptr = unsafe { NonNull::new_unchecked(recorder as *const _ as *mut _) }; + let recorder_ptr = unsafe { NonNull::new_unchecked(recorder_ptr) }; let prev_recorder = LOCAL_RECORDER.with(|local_recorder| local_recorder.replace(Some(recorder_ptr))); From 355da553ad404edd3ade2abafc0e54b318b339e4 Mon Sep 17 00:00:00 2001 From: Boxy Date: Thu, 27 Mar 2025 17:52:47 +0000 Subject: [PATCH 2/2] Make lifetimes explicit and document safety --- metrics/src/recorder/mod.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/metrics/src/recorder/mod.rs b/metrics/src/recorder/mod.rs index b2f61538..5653ace0 100644 --- a/metrics/src/recorder/mod.rs +++ b/metrics/src/recorder/mod.rs @@ -139,11 +139,13 @@ pub struct LocalRecorderGuard<'a> { impl<'a> LocalRecorderGuard<'a> { /// Creates a new `LocalRecorderGuard` and sets the thread-local recorder. - fn new(recorder: &'a dyn Recorder) -> Self { - // SAFETY: Why this it okay to extend the lifetime of the object type from `'a` to `'static`? + fn new(recorder: &'a (dyn Recorder + 'a)) -> Self { + // SAFETY: We extend `'a` to `'static` to satisfy the signature of `LOCAL_RECORDER`, which + // has an implied `'static` bound on `dyn Recorder`. We enforce that all usages of `LOCAL_RECORDER` + // are limited to `'a` as we mediate its access entirely through `LocalRecorderGuard<'a>`. let recorder_ptr = unsafe { - std::mem::transmute::<*const (dyn Recorder + '_), *mut (dyn Recorder + 'static)>( - recorder as &dyn Recorder, + std::mem::transmute::<*const (dyn Recorder + 'a), *mut (dyn Recorder + 'static)>( + recorder as &'a (dyn Recorder + 'a), ) }; // SAFETY: While we take a lifetime-less pointer to the given reference, the reference we derive _from_ the