From 2bc2fd9cfd722b354a905df737cb92643aabfd13 Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Mon, 18 Mar 2024 16:12:01 +0100 Subject: [PATCH] fixup(#7160 / tokio_epoll_uring_ext): double-panic caused by info! in thread-local's drop() (#7164) Manual testing of the changes in #7160 revealed that, if the thread-local destructor ever runs (it apparently doesn't in our test suite runs, otherwise #7160 would not have auto-merged), we can encounter an `abort()` due to a double-panic in the tracing code. This github comment here contains the stack trace: https://github.com/neondatabase/neon/pull/7160#issuecomment-2003778176 This PR reverts #7160 and uses a atomic counter to identify the thread-local in log messages, instead of the memory address of the thread local, which may be re-used. --- .../io_engine/tokio_epoll_uring_ext.rs | 29 +++++-------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/pageserver/src/virtual_file/io_engine/tokio_epoll_uring_ext.rs b/pageserver/src/virtual_file/io_engine/tokio_epoll_uring_ext.rs index 9b2efef5d4..6ea19d6b2d 100644 --- a/pageserver/src/virtual_file/io_engine/tokio_epoll_uring_ext.rs +++ b/pageserver/src/virtual_file/io_engine/tokio_epoll_uring_ext.rs @@ -5,8 +5,8 @@ //! on older kernels, such as some (but not all) older kernels in the Linux 5.10 series. //! See for more details. -use std::sync::atomic::AtomicU32; -use std::sync::{Arc, Weak}; +use std::sync::atomic::{AtomicU32, AtomicU64, Ordering}; +use std::sync::Arc; use tokio_util::sync::CancellationToken; use tracing::{error, info, info_span, warn, Instrument}; @@ -24,38 +24,25 @@ struct ThreadLocalState(Arc); struct ThreadLocalStateInner { cell: tokio::sync::OnceCell, launch_attempts: AtomicU32, - weak_self: Weak, + /// populated through fetch_add from [`THREAD_LOCAL_STATE_ID`] + thread_local_state_id: u64, } impl ThreadLocalState { pub fn new() -> Self { - Self(Arc::new_cyclic(|weak| ThreadLocalStateInner { + Self(Arc::new(ThreadLocalStateInner { cell: tokio::sync::OnceCell::default(), launch_attempts: AtomicU32::new(0), - weak_self: Weak::clone(weak), + thread_local_state_id: THREAD_LOCAL_STATE_ID.fetch_add(1, Ordering::Relaxed), })) } -} -impl ThreadLocalStateInner { pub fn make_id_string(&self) -> String { - format!("0x{:p}", self.weak_self.as_ptr()) + format!("{}", self.0.thread_local_state_id) } } -impl Drop for ThreadLocalStateInner { - fn drop(&mut self) { - info!(parent: None, id=%self.make_id_string(), "tokio_epoll_uring_ext: thread-local state is being dropped and id might be re-used in the future"); - } -} - -impl std::ops::Deref for ThreadLocalState { - type Target = ThreadLocalStateInner; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} +static THREAD_LOCAL_STATE_ID: AtomicU64 = AtomicU64::new(0); thread_local! { static THREAD_LOCAL: ThreadLocalState = ThreadLocalState::new();