From cb954319901286ce38bbe078ee99d4f57af5986a Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Thu, 23 Apr 2026 11:54:01 +0800 Subject: [PATCH] Rationalize LazyCell/LazyLock/OnceCell/OnceLock usage - Prefer using Lazy* for things that can be initialized by a "fn()", i.e. that don't depend on non-static data. Rationale: communicate that we don't need the flexibility that comes with Once*. - Prefer using SingleThreaded (or MainThread) LazyCell/OnceCell rather than LazyLock/OnceLock. To communicate that these statics don't cross thread boundaries. Incomplete. Not yet sure if this is a good idea. --- Cargo.lock | 7 +++ Cargo.toml | 2 + crates/common/Cargo.toml | 1 + crates/common/src/lib.rs | 10 ++-- crates/thread/Cargo.toml | 14 +++++ crates/thread/src/lib.rs | 98 ++++++++++++++++++++++++++++++++++ crates/xtask/Cargo.toml | 1 + crates/xtask/src/shellcheck.rs | 6 +-- src/common.rs | 4 +- src/env/config_paths.rs | 8 +-- src/env/environment.rs | 22 ++++---- src/env/environment_impl.rs | 10 ++-- src/env/var.rs | 8 +-- src/exec.rs | 8 +-- src/operation_context.rs | 9 ++-- src/panic.rs | 9 ++-- src/reader/reader.rs | 6 ++- src/terminal.rs | 6 +-- src/tests/prelude.rs | 1 - src/threads/threads.rs | 59 +------------------- src/tty_handoff.rs | 16 +++--- src/universal_notifier/mod.rs | 12 +++-- 22 files changed, 195 insertions(+), 122 deletions(-) create mode 100644 crates/thread/Cargo.toml create mode 100644 crates/thread/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 218725a4e..0ffa9c853 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -303,6 +303,7 @@ dependencies = [ "fish-gettext-mo-file-parser", "fish-printf", "fish-tempfile", + "fish-thread", "fish-util", "fish-wcstringutil", "fish-wgetopt", @@ -358,6 +359,7 @@ dependencies = [ "bitflags", "fish-build-helper", "fish-feature-flags", + "fish-thread", "fish-widestring", "libc", "nix", @@ -435,6 +437,10 @@ dependencies = [ "rand", ] +[[package]] +name = "fish-thread" +version = "0.0.0" + [[package]] name = "fish-util" version = "0.0.0" @@ -1236,6 +1242,7 @@ dependencies = [ "clap", "fish-build-helper", "fish-tempfile", + "fish-thread", "ignore", "pcre2", "walkdir", diff --git a/Cargo.toml b/Cargo.toml index 023ea2d6d..b0eb557df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,7 @@ fish-gettext-maps = { path = "crates/gettext-maps" } fish-gettext-mo-file-parser = { path = "crates/gettext-mo-file-parser" } fish-printf = { path = "crates/printf", features = ["widestring"] } fish-tempfile = { path = "crates/tempfile" } +fish-thread = { path = "crates/thread" } fish-util = { path = "crates/util" } fish-wcstringutil = { path = "crates/wcstringutil" } fish-widecharwidth = { path = "crates/widecharwidth" } @@ -117,6 +118,7 @@ fish-gettext = { workspace = true, optional = true } fish-gettext-extraction = { workspace = true, optional = true } fish-printf.workspace = true fish-tempfile.workspace = true +fish-thread.workspace = true fish-util.workspace = true fish-wcstringutil.workspace = true fish-wgetopt.workspace = true diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml index 9afd33b84..ed4bce01f 100644 --- a/crates/common/Cargo.toml +++ b/crates/common/Cargo.toml @@ -10,6 +10,7 @@ license.workspace = true bitflags.workspace = true fish-feature-flags.workspace = true fish-widestring.workspace = true +fish-thread.workspace = true libc.workspace = true nix.workspace = true diff --git a/crates/common/src/lib.rs b/crates/common/src/lib.rs index c1df7e530..278a3ff0b 100644 --- a/crates/common/src/lib.rs +++ b/crates/common/src/lib.rs @@ -1,5 +1,6 @@ use bitflags::bitflags; use fish_feature_flags::{FeatureFlag, feature_test}; +use fish_thread::SingleThreadedLazyCell; use fish_widestring::{ ANY_CHAR, ANY_STRING, ANY_STRING_RECURSIVE, ASCII_MAX, BRACE_BEGIN, BRACE_END, BRACE_SEP, BRACE_SPACE, BYTE_MAX, HOME_DIRECTORY, INTERNAL_SEPARATOR, L, PROCESS_EXPAND_SELF, @@ -18,7 +19,7 @@ unix::ffi::OsStrExt as _, }, sync::{ - Arc, OnceLock, + Arc, atomic::{AtomicI32, AtomicU32, Ordering}, }, time, @@ -1018,9 +1019,8 @@ pub fn read_unquoted_escape( /// session. We err on the side of assuming it's not a console session. This approach isn't /// bullet-proof and that's OK. pub fn is_console_session() -> bool { - static IS_CONSOLE_SESSION: OnceLock = OnceLock::new(); // TODO(terminal-workaround) - *IS_CONSOLE_SESSION.get_or_init(|| { + static IS_CONSOLE_SESSION: SingleThreadedLazyCell = SingleThreadedLazyCell::new(|| // No console session on Apple, and ttyname may hang (#12506). !cfg!(apple) && nix::unistd::ttyname(unsafe { std::os::fd::BorrowedFd::borrow_raw(STDIN_FILENO) }) @@ -1037,8 +1037,8 @@ pub fn is_console_session() -> bool { // and that $TERM is simple, e.g. `xterm` or `vt100`, not `xterm-something` or `sun-color`. is_console_tty && env::var_os("TERM").is_none_or(|t| !t.as_bytes().contains(&b'-')) - }) - }) + })); + *IS_CONSOLE_SESSION } /// Exits without invoking destructors (via _exit), useful for code after fork. diff --git a/crates/thread/Cargo.toml b/crates/thread/Cargo.toml new file mode 100644 index 000000000..b546022ee --- /dev/null +++ b/crates/thread/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "fish-thread" +edition.workspace = true +rust-version.workspace = true +version = "0.0.0" +repository.workspace = true +license.workspace = true + +[dependencies] + +[build-dependencies] + +[lints] +workspace = true diff --git a/crates/thread/src/lib.rs b/crates/thread/src/lib.rs new file mode 100644 index 000000000..3434702f4 --- /dev/null +++ b/crates/thread/src/lib.rs @@ -0,0 +1,98 @@ +use core::{cell::OnceCell, marker::PhantomData}; +use std::{ + cell::LazyCell, + sync::atomic::{AtomicUsize, Ordering}, +}; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub struct ThreadId(usize); + +/// Get the calling thread's fish-specific thread id. +/// +/// This thread id is internal to the `threads` module for low-level purposes and should not be +/// leaked to other modules; general purpose code that needs a thread id should use rust's native +/// thread id functionality. +/// +/// We use our own implementation because Rust's own `Thread::id()` allocates via `Arc`, is fairly +/// slow, and uses a `Mutex` on 32-bit platforms (or anywhere without an atomic 64-bit CAS). +#[inline(always)] +pub fn thread_id() -> ThreadId { + static THREAD_COUNTER: AtomicUsize = AtomicUsize::new(1); + // It would be faster and much nicer to use #[thread_local] here, but that's nightly only. + // This is still faster than going through Thread::thread_id(); it's something like 15ns + // for each `Thread::thread_id()` call vs 1-2 ns with `#[thread_local]` and 2-4ns with + // `thread_local!`. + thread_local! { + static THREAD_ID: ThreadId = ThreadId(THREAD_COUNTER.fetch_add(1, Ordering::Relaxed)); + } + let id = THREAD_ID.with(|id| *id); + // This assertion is only here to reduce hair loss in case someone runs into a known linker bug; + // as it's not here to catch logic errors in our own code, it can be elided in release mode. + debug_assert_ne!(id, ThreadId(0), "TLS storage not initialized!"); + id +} + +/// A `Sync` and `Send` wrapper for non-`Sync`/`Send` types. +/// Only allows access from one thread. +pub struct SingleThreaded { + thread_id: OnceCell, + data: T, + // Make type !Send and !Sync by default + _marker: PhantomData<*const ()>, +} + +// Can be shared across threads as long as T is 'static. +unsafe impl Send for SingleThreaded {} +unsafe impl Sync for SingleThreaded {} + +impl SingleThreaded { + pub const fn new(value: T) -> Self { + Self { + thread_id: OnceCell::new(), + data: value, + _marker: PhantomData, + } + } + + pub fn get(&self) -> &T { + assert!(thread_id() == *self.thread_id.get_or_init(thread_id)); + &self.data + } +} + +impl std::ops::Deref for SingleThreaded { + type Target = T; + fn deref(&self) -> &Self::Target { + self.get() + } +} + +pub struct SingleThreadedLazyCell T>(SingleThreaded>); + +impl T> SingleThreadedLazyCell { + pub const fn new(f: F) -> Self { + Self(SingleThreaded::new(LazyCell::new(f))) + } +} + +impl T> std::ops::Deref for SingleThreadedLazyCell { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +pub struct SingleThreadedOnceCell(SingleThreaded>); + +impl SingleThreadedOnceCell { + pub const fn new() -> Self { + Self(SingleThreaded::new(OnceCell::new())) + } +} + +impl std::ops::Deref for SingleThreadedOnceCell { + type Target = OnceCell; + fn deref(&self) -> &Self::Target { + &self.0 + } +} diff --git a/crates/xtask/Cargo.toml b/crates/xtask/Cargo.toml index 66c92aaee..17cdef87c 100644 --- a/crates/xtask/Cargo.toml +++ b/crates/xtask/Cargo.toml @@ -10,6 +10,7 @@ anstyle.workspace = true clap.workspace = true fish-build-helper.workspace = true fish-tempfile.workspace = true +fish-thread.workspace = true ignore.workspace = true pcre2.workspace = true walkdir.workspace = true diff --git a/crates/xtask/src/shellcheck.rs b/crates/xtask/src/shellcheck.rs index 713b6a2c6..1adf0f483 100644 --- a/crates/xtask/src/shellcheck.rs +++ b/crates/xtask/src/shellcheck.rs @@ -1,4 +1,5 @@ use fish_build_helper::workspace_root; +use fish_thread::SingleThreadedLazyCell; use ignore::Walk; use pcre2::bytes::Regex; use std::{ @@ -6,7 +7,6 @@ io::{BufRead, BufReader}, path::{Path, PathBuf}, process::Command, - sync::OnceLock, }; pub fn shellcheck() { @@ -32,9 +32,9 @@ fn is_shell_script>(path: P) -> bool { let Ok(_) = BufReader::new(file).read_line(&mut first_line) else { return false; }; - static SHEBANG_REGEX: OnceLock = OnceLock::new(); + static SHEBANG_REGEX: SingleThreadedLazyCell = + SingleThreadedLazyCell::new(|| Regex::new("^#!.*[^i]sh").unwrap()); SHEBANG_REGEX - .get_or_init(|| Regex::new("^#!.*[^i]sh").unwrap()) .is_match(first_line.trim().as_bytes()) .unwrap() } diff --git a/src/common.rs b/src/common.rs index 9c35571cd..00b64e2c4 100644 --- a/src/common.rs +++ b/src/common.rs @@ -213,8 +213,8 @@ pub fn is_windows_subsystem_for_linux(_: WSL) -> bool { /// See and [Microsoft/WSL#2997](https://github.com/Microsoft/WSL/issues/2997) #[cfg(target_os = "linux")] pub fn is_windows_subsystem_for_linux(v: WSL) -> bool { - use std::sync::OnceLock; - static RESULT: OnceLock> = OnceLock::new(); + use fish_thread::SingleThreadedOnceCell; + static RESULT: SingleThreadedOnceCell> = SingleThreadedOnceCell::new(); // This is called post-fork from [`report_setpgid_error()`], so the fast path must not involve // any allocations or mutexes. We can't rely on all the std functions to be alloc-free in both diff --git a/src/env/config_paths.rs b/src/env/config_paths.rs index d7520f984..92680782d 100644 --- a/src/env/config_paths.rs +++ b/src/env/config_paths.rs @@ -1,10 +1,10 @@ use crate::common::{BUILD_DIR, get_program_name}; use crate::{flog, flogf}; use fish_build_helper::workspace_root; +use fish_thread::SingleThreadedLazyCell; use std::ffi::OsStr; use std::os::unix::ffi::OsStrExt as _; use std::path::{Path, PathBuf}; -use std::sync::OnceLock; /// A struct of configuration directories, determined in main() that fish will optionally pass to /// env_init. @@ -22,7 +22,6 @@ pub struct ConfigPaths { impl ConfigPaths { pub fn new() -> Self { - FISH_PATH.get_or_init(compute_fish_path); let exec_path = get_fish_path(); flog!( config, @@ -167,11 +166,12 @@ pub enum FishPath { LookUpInPath, } -static FISH_PATH: OnceLock = OnceLock::new(); +static FISH_PATH: SingleThreadedLazyCell FishPath> = + SingleThreadedLazyCell::new(compute_fish_path); /// Get the absolute path to the fish executable itself pub fn get_fish_path() -> &'static FishPath { - FISH_PATH.get().unwrap() + &FISH_PATH } fn compute_fish_path() -> FishPath { diff --git a/src/env/environment.rs b/src/env/environment.rs index d00e3563a..5e999c8f1 100644 --- a/src/env/environment.rs +++ b/src/env/environment.rs @@ -30,6 +30,7 @@ wutil::{fish_wcstol, wgetcwd}, }; use fish_common::{UnescapeStringStyle, unescape_string}; +use fish_thread::{SingleThreadedLazyCell, SingleThreadedOnceCell}; use fish_wcstringutil::join_strings; use fish_widestring::{cstr2wcstring, osstr2wcstring, str2wcstring}; use libc::c_int; @@ -41,7 +42,7 @@ collections::HashMap, ffi::CStr, path::PathBuf, - sync::{Arc, LazyLock, OnceLock}, + sync::{Arc, LazyLock}, }; /// Set when a universal variable has been modified but not yet been written to disk via sync(). @@ -405,14 +406,14 @@ pub fn universal_sync(&self, always: bool, is_repainting: bool) -> Vec { /// A variable stack that only represents globals. /// Do not push or pop from this. pub fn globals() -> &'static EnvStack { - use std::sync::OnceLock; - static GLOBALS: OnceLock = OnceLock::new(); - GLOBALS.get_or_init(|| EnvStack { - inner: EnvStackImpl::new(), - can_push_pop: false, - // Do not dispatch variable changes - this is used at startup when we are importing env vars. - dispatches_var_changes: false, - }) + static GLOBALS: SingleThreadedLazyCell = + SingleThreadedLazyCell::new(|| EnvStack { + inner: EnvStackImpl::new(), + can_push_pop: false, + // Do not dispatch variable changes - this is used at startup when we are importing env vars. + dispatches_var_changes: false, + }); + &GLOBALS } pub fn set_argv(&self, argv: Vec, is_repainting: bool) { @@ -562,7 +563,8 @@ fn setup_path(global_exported_mode: EnvSetMode) { /// The originally inherited variables and their values. /// This is a simple key->value map and not e.g. cut into paths. -pub static INHERITED_VARS: OnceLock> = OnceLock::new(); +pub static INHERITED_VARS: SingleThreadedOnceCell> = + SingleThreadedOnceCell::new(); pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool) { let vars = EnvStack::globals(); diff --git a/src/env/environment_impl.rs b/src/env/environment_impl.rs index 6e040e79a..7c9725c0b 100644 --- a/src/env/environment_impl.rs +++ b/src/env/environment_impl.rs @@ -13,6 +13,7 @@ use crate::reader::{commandline_get_state, reader_status_count}; use crate::threads::{is_forked_child, is_main_thread}; use crate::wutil::fish_wcstol_radix; +use fish_thread::SingleThreadedLazyCell; use fish_widestring::wcs2zstring; use nix::sys::stat::{Mode, umask}; use std::cell::{RefCell, UnsafeCell}; @@ -27,13 +28,10 @@ /// Getter for universal variables. /// This is typically initialized in env_init(), and is considered empty before then. pub fn uvars() -> MutexGuard<'static, EnvUniversal> { - use std::sync::OnceLock; /// Universal variables instance. - static UVARS: OnceLock> = OnceLock::new(); - UVARS - .get_or_init(|| Mutex::new(EnvUniversal::new())) - .lock() - .unwrap() + static UVARS: SingleThreadedLazyCell> = + SingleThreadedLazyCell::new(|| Mutex::new(EnvUniversal::new())); + UVARS.lock().unwrap() } /// Whether we were launched with no_config; in this case setting a uvar instead sets a global. diff --git a/src/env/var.rs b/src/env/var.rs index 48a6a1635..399187981 100644 --- a/src/env/var.rs +++ b/src/env/var.rs @@ -1,6 +1,7 @@ use crate::signal::Signal; use bitflags::bitflags; use fish_common::assert_sorted_by_name; +use fish_thread::SingleThreadedLazyCell; use fish_wcstringutil::join_strings; use fish_widestring::{L, WString, wstr}; use libc::c_int; @@ -130,13 +131,12 @@ pub struct EnvVar { impl Default for EnvVar { fn default() -> Self { - use std::sync::OnceLock; /// A shared read-only empty list. - static EMPTY_LIST: OnceLock> = OnceLock::new(); - let empty_list = EMPTY_LIST.get_or_init(|| Arc::new([])); + static EMPTY_LIST: SingleThreadedLazyCell> = + SingleThreadedLazyCell::new(|| Arc::new([])); EnvVar { - values: Arc::clone(empty_list), + values: Arc::clone(&*EMPTY_LIST), flags: EnvVarFlags::empty(), } } diff --git a/src/exec.rs b/src/exec.rs index b50605f96..3c29de5b8 100644 --- a/src/exec.rs +++ b/src/exec.rs @@ -44,6 +44,7 @@ use crate::wutil::{fish_wcstol, perror_io}; use errno::{errno, set_errno}; use fish_common::{ScopeGuard, exit_without_destructors, truncate_at_nul, write_loop}; +use fish_thread::SingleThreadedLazyCell; use fish_widestring::{ToWString as _, bytes2wcstring, wcs2bytes, wcs2zstring}; use libc::{ EACCES, ENOENT, ENOEXEC, ENOTDIR, EPIPE, EXIT_FAILURE, EXIT_SUCCESS, STDERR_FILENO, @@ -62,7 +63,7 @@ os::fd::{AsRawFd as _, FromRawFd as _, OwnedFd, RawFd}, slice, sync::{ - Arc, OnceLock, + Arc, atomic::{AtomicUsize, Ordering}, }, }; @@ -72,9 +73,10 @@ /// to their target fds. /// TODO: this IO could be multiplexed using FdMonitor. fn exec_thread_pool() -> &'static Arc { - static EXEC_THREAD_POOL: OnceLock> = OnceLock::new(); + static EXEC_THREAD_POOL: SingleThreadedLazyCell> = + SingleThreadedLazyCell::new(|| ThreadPool::new(1, usize::MAX)); // Use an unbounded queue because otherwise we risk deadlock. - EXEC_THREAD_POOL.get_or_init(|| ThreadPool::new(1, usize::MAX)) + &EXEC_THREAD_POOL } /// Execute the processes specified by `j` in the parser \p. diff --git a/src/operation_context.rs b/src/operation_context.rs index 34995f9b4..fd088504c 100644 --- a/src/operation_context.rs +++ b/src/operation_context.rs @@ -1,3 +1,5 @@ +use fish_thread::SingleThreadedLazyCell; + use crate::common::CancelChecker; use crate::env::EnvDyn; use crate::env::{EnvStack, Environment}; @@ -57,10 +59,9 @@ pub fn vars(&self) -> &dyn Environment { // Return an "empty" context which contains no variables, no parser, and never cancels. pub fn empty() -> OperationContext<'static> { - use std::sync::OnceLock; - static NULL_ENV: OnceLock = OnceLock::new(); - let null_env = NULL_ENV.get_or_init(EnvStack::new); - OperationContext::background(null_env, EXPANSION_LIMIT_DEFAULT) + static NULL_ENV: SingleThreadedLazyCell EnvStack> = + SingleThreadedLazyCell::new(EnvStack::new); + OperationContext::background(&*NULL_ENV, EXPANSION_LIMIT_DEFAULT) } // Return an operation context that contains only global variables, no parser, and never diff --git a/src/panic.rs b/src/panic.rs index abb45d397..35ab85ba7 100644 --- a/src/panic.rs +++ b/src/panic.rs @@ -1,16 +1,15 @@ use crate::{common::get_program_name, nix::isatty, threads::is_main_thread}; use fish_common::read_blocked; +use fish_thread::SingleThreadedOnceCell; use libc::STDIN_FILENO; use std::{ panic::{UnwindSafe, set_hook, take_hook}, - sync::{ - OnceLock, - atomic::{AtomicBool, Ordering}, - }, + sync::atomic::{AtomicBool, Ordering}, time::Duration, }; -pub static AT_EXIT: OnceLock> = OnceLock::new(); +pub static AT_EXIT: SingleThreadedOnceCell> = + SingleThreadedOnceCell::new(); pub fn panic_handler(main: impl FnOnce() -> i32 + UnwindSafe) -> ! { // The isatty() check will stop us from hanging in most fish tests, but not those diff --git a/src/reader/reader.rs b/src/reader/reader.rs index d7bd38996..c7dea7bbb 100644 --- a/src/reader/reader.rs +++ b/src/reader/reader.rs @@ -117,6 +117,7 @@ }; use fish_fallback::{fish_wcwidth, lowercase}; use fish_feature_flags::FeatureFlag; +use fish_thread::SingleThreadedOnceCell; use fish_util::{perror, write_to_fd}; use fish_wcstringutil::{ CaseSensitivity, IsPrefix, StringFuzzyMatch, count_preceding_backslashes, is_prefix, @@ -147,7 +148,7 @@ os::fd::{AsRawFd as _, BorrowedFd, FromRawFd as _, OwnedFd, RawFd}, pin::Pin, sync::{ - Arc, LazyLock, Mutex, MutexGuard, OnceLock, + Arc, LazyLock, Mutex, MutexGuard, atomic::{AtomicI32, AtomicU8, AtomicU32, Ordering}, }, time::{Duration, Instant}, @@ -174,7 +175,8 @@ fn zeroed_termios() -> Termios { pub static SHELL_MODES: LazyLock> = LazyLock::new(|| Mutex::new(zeroed_termios())); /// The valid terminal modes on startup. -static TERMINAL_MODE_ON_STARTUP: OnceLock = OnceLock::new(); +static TERMINAL_MODE_ON_STARTUP: SingleThreadedOnceCell = + SingleThreadedOnceCell::new(); /// Mode we use to execute programs. static TTY_MODES_FOR_EXTERNAL_CMDS: LazyLock> = diff --git a/src/terminal.rs b/src/terminal.rs index d5633aab8..61a669789 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -4,12 +4,12 @@ use crate::{ screen::{is_dumb, only_grayscale}, text_face::{ResettableStyle, TextFace, TextStyling, UnderlineStyle}, - threads::MainThread, }; use bitflags::bitflags; use fish_color::{Color, Color24}; use fish_common::{EscapeStringStyle, escape_string, write_loop}; use fish_feature_flags::FeatureFlag; +use fish_thread::SingleThreaded; use fish_widestring::{wcs2bytes, wcs2bytes_appending}; use std::{ cell::{RefCell, RefMut}, @@ -575,8 +575,8 @@ pub fn write_bytes(&mut self, buf: &[u8]) { /// Access the outputter for stdout. /// This should only be used from the main thread. pub fn stdoutput() -> &'static RefCell { - static STDOUTPUT: MainThread> = - MainThread::new(RefCell::new(Outputter::new_from_fd(libc::STDOUT_FILENO))); + static STDOUTPUT: SingleThreaded> = + SingleThreaded::new(RefCell::new(Outputter::new_from_fd(libc::STDOUT_FILENO))); STDOUTPUT.get() } } diff --git a/src/tests/prelude.rs b/src/tests/prelude.rs index 101cbe5f3..6abd778a7 100644 --- a/src/tests/prelude.rs +++ b/src/tests/prelude.rs @@ -12,7 +12,6 @@ use std::collections::HashMap; use std::env::set_current_dir; use std::path::PathBuf; -use std::sync::OnceLock; pub use serial_test::serial; diff --git a/src/threads/threads.rs b/src/threads/threads.rs index 897d0004b..f10104392 100644 --- a/src/threads/threads.rs +++ b/src/threads/threads.rs @@ -1,14 +1,11 @@ //! Support for thread pools and thread management. use crate::flog::{FloggableDebug, flog}; +use fish_thread::{ThreadId, thread_id}; use nix::sys::signal::{SigSet, SigmaskHow, Signal}; -use std::marker::PhantomData; -use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex, OnceLock}; use std::time::Duration; -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub struct ThreadId(usize); - impl FloggableDebug for ThreadId {} impl FloggableDebug for std::thread::ThreadId {} @@ -55,31 +52,6 @@ fn init_not_called() -> ! { } } -/// Get the calling thread's fish-specific thread id. -/// -/// This thread id is internal to the `threads` module for low-level purposes and should not be -/// leaked to other modules; general purpose code that needs a thread id should use rust's native -/// thread id functionality. -/// -/// We use our own implementation because Rust's own `Thread::id()` allocates via `Arc`, is fairly -/// slow, and uses a `Mutex` on 32-bit platforms (or anywhere without an atomic 64-bit CAS). -#[inline(always)] -fn thread_id() -> ThreadId { - static THREAD_COUNTER: AtomicUsize = AtomicUsize::new(1); - // It would be faster and much nicer to use #[thread_local] here, but that's nightly only. - // This is still faster than going through Thread::thread_id(); it's something like 15ns - // for each `Thread::thread_id()` call vs 1-2 ns with `#[thread_local]` and 2-4ns with - // `thread_local!`. - thread_local! { - static THREAD_ID: ThreadId = ThreadId(THREAD_COUNTER.fetch_add(1, Ordering::Relaxed)); - } - let id = THREAD_ID.with(|id| *id); - // This assertion is only here to reduce hair loss in case someone runs into a known linker bug; - // as it's not here to catch logic errors in our own code, it can be elided in release mode. - debug_assert_ne!(id, ThreadId(0), "TLS storage not initialized!"); - id -} - #[inline(always)] pub fn is_main_thread() -> bool { thread_id() == main_thread_id() @@ -295,33 +267,6 @@ fn spawn_thread(self: &Arc) -> bool { } } -/// A `Sync` and `Send` wrapper for non-`Sync`/`Send` types. -/// Only allows access from the main thread. -pub struct MainThread { - data: T, - // Make type !Send and !Sync by default - _marker: PhantomData<*const ()>, -} - -// Manually implement Send and Sync for MainThread to ensure it can be shared across threads -// as long as T is 'static. -unsafe impl Send for MainThread {} -unsafe impl Sync for MainThread {} - -impl MainThread { - pub const fn new(value: T) -> Self { - Self { - data: value, - _marker: PhantomData, - } - } - - pub fn get(&self) -> &T { - assert_is_main_thread(); - &self.data - } -} - impl ThreadPool { /// The worker loop entry point for this thread. /// This is run in a background thread. diff --git a/src/tty_handoff.rs b/src/tty_handoff.rs index 846be780f..4e62a8264 100644 --- a/src/tty_handoff.rs +++ b/src/tty_handoff.rs @@ -18,25 +18,24 @@ use crate::threads::assert_is_main_thread; use crate::wutil::{perror_nix, wcstoi}; use fish_common::write_loop; +use fish_thread::SingleThreadedOnceCell; use fish_util::perror; use libc::{EINVAL, ENOTTY, EPERM, STDIN_FILENO, WNOHANG}; use nix::sys::termios::tcgetattr; use nix::unistd::getpgrp; use std::os::fd::BorrowedFd; -use std::sync::{ - OnceLock, - atomic::{AtomicPtr, Ordering}, -}; +use std::sync::atomic::{AtomicPtr, Ordering}; /// Whether kitty keyboard protocol support is present in the TTY. -static KITTY_KEYBOARD_SUPPORTED: OnceLock = OnceLock::new(); +static KITTY_KEYBOARD_SUPPORTED: SingleThreadedOnceCell = SingleThreadedOnceCell::new(); /// Set that the TTY supports the kitty keyboard protocol. pub fn maybe_set_kitty_keyboard_capability() { KITTY_KEYBOARD_SUPPORTED.get_or_init(|| true); } -pub(crate) static SCROLL_CONTENT_UP_SUPPORTED: OnceLock = OnceLock::new(); +pub(crate) static SCROLL_CONTENT_UP_SUPPORTED: SingleThreadedOnceCell = + SingleThreadedOnceCell::new(); pub(crate) const SCROLL_CONTENT_UP_TERMINFO_CODE: &str = "indn"; // Get the support capability for kitty keyboard protocol. @@ -51,10 +50,11 @@ pub fn maybe_set_scroll_content_up_capability() { }); } -pub static TERMINAL_OS_NAME: OnceLock> = OnceLock::new(); +pub static TERMINAL_OS_NAME: SingleThreadedOnceCell> = + SingleThreadedOnceCell::new(); pub(crate) const XTGETTCAP_QUERY_OS_NAME: &str = "query-os-name"; -pub static XTVERSION: OnceLock = OnceLock::new(); +pub static XTVERSION: SingleThreadedOnceCell = SingleThreadedOnceCell::new(); pub fn xtversion() -> Option<&'static wstr> { XTVERSION.get().as_ref().map(|s| s.as_utfstr()) diff --git a/src/universal_notifier/mod.rs b/src/universal_notifier/mod.rs index 40262ae3d..64a679216 100644 --- a/src/universal_notifier/mod.rs +++ b/src/universal_notifier/mod.rs @@ -1,4 +1,6 @@ -use std::{os::fd::RawFd, sync::OnceLock}; +use std::os::fd::RawFd; + +use fish_thread::SingleThreadedLazyCell; #[cfg(apple)] mod notifyd; @@ -67,9 +69,9 @@ pub fn create_notifier() -> Box { Box::new(NullNotifier) } -// Default instance. Other instances are possible for testing. -static DEFAULT_NOTIFIER: OnceLock> = OnceLock::new(); - pub fn default_notifier() -> &'static dyn UniversalNotifier { - DEFAULT_NOTIFIER.get_or_init(create_notifier).as_ref() + // Default instance. Other instances are possible for testing. + static DEFAULT_NOTIFIER: SingleThreadedLazyCell> = + SingleThreadedLazyCell::new(create_notifier); + &**DEFAULT_NOTIFIER }