From 45e249dd94d7b77c0fe4373d64ad5c4612af70b2 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Thu, 16 May 2024 21:07:41 -0500 Subject: [PATCH] Revert removal of Arc from principal() and global() This reverts commit c6d3bde0c6bb87291322576b57202784382653fb. This reverts commit 4ce13f0adbb54396512664b6054808d324eb3a15. --- src/bin/fish_indent.rs | 2 +- src/complete.rs | 2 +- src/env/environment.rs | 16 ++++++++-------- src/function.rs | 2 +- src/operation_context.rs | 2 +- src/parser.rs | 19 ++++++++++++------- 6 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/bin/fish_indent.rs b/src/bin/fish_indent.rs index 66e0cd6fb..6d19b7687 100644 --- a/src/bin/fish_indent.rs +++ b/src/bin/fish_indent.rs @@ -990,7 +990,7 @@ enum OutputType { } } OutputType::Ansi => { - colored_output = colorize(&output_wtext, &colors, EnvStack::globals()); + colored_output = colorize(&output_wtext, &colors, &**EnvStack::globals()); } OutputType::Html => { colored_output = html_colorize(&output_wtext, &colors); diff --git a/src/complete.rs b/src/complete.rs index 8d129384f..877eef75c 100644 --- a/src/complete.rs +++ b/src/complete.rs @@ -2473,7 +2473,7 @@ pub fn complete_load(cmd: &wstr, parser: &Parser) -> bool { let path_to_load = completion_autoloader .lock() .expect("mutex poisoned") - .resolve_command(cmd, EnvStack::globals()); + .resolve_command(cmd, &**EnvStack::globals()); if let Some(path_to_load) = path_to_load { Autoload::perform_autoload(&path_to_load, parser); completion_autoloader diff --git a/src/env/environment.rs b/src/env/environment.rs index ed62f1e46..86704bbf4 100644 --- a/src/env/environment.rs +++ b/src/env/environment.rs @@ -188,7 +188,7 @@ fn lock(&self) -> EnvMutexGuard { /// Return whether we are the principal stack. pub fn is_principal(&self) -> bool { - std::ptr::eq(self, Self::principal()) + std::ptr::eq(self, &**Self::principal()) } /// Helpers to get and set the proc statuses. @@ -360,17 +360,17 @@ pub fn universal_sync(&self, always: bool) -> Vec { /// A variable stack that only represents globals. /// Do not push or pop from this. - pub fn globals() -> &'static EnvStack { + pub fn globals() -> &'static Arc { use std::sync::OnceLock; - static GLOBALS: OnceLock = OnceLock::new(); - GLOBALS.get_or_init(|| EnvStack::new()) + static GLOBALS: OnceLock> = OnceLock::new(); + &GLOBALS.get_or_init(|| Arc::new(EnvStack::new())) } /// Access the principal variable stack, associated with the principal parser. - pub fn principal() -> &'static EnvStack { + pub fn principal() -> &'static Arc { use std::sync::OnceLock; - static PRINCIPAL_STACK: OnceLock = OnceLock::new(); - &PRINCIPAL_STACK.get_or_init(|| EnvStack::new()) + static PRINCIPAL_STACK: OnceLock> = OnceLock::new(); + &PRINCIPAL_STACK.get_or_init(|| Arc::new(EnvStack::new())) } pub fn set_argv(&self, argv: Vec) { @@ -554,7 +554,7 @@ fn setup_path() { pub static INHERITED_VARS: OnceCell> = OnceCell::new(); pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool) { - let vars = EnvStack::principal(); + let vars = &**EnvStack::principal(); let env_iter: Vec<_> = std::env::vars_os() .map(|(k, v)| (str2wcstring(k.as_bytes()), str2wcstring(v.as_bytes()))) diff --git a/src/function.rs b/src/function.rs index cef313d72..008ebe730 100644 --- a/src/function.rs +++ b/src/function.rs @@ -122,7 +122,7 @@ pub fn load(name: &wstr, parser: &Parser) -> bool { if funcset.allow_autoload(name) { if let Some(path) = funcset .autoloader - .resolve_command(name, EnvStack::globals()) + .resolve_command(name, &**EnvStack::globals()) { path_to_autoload = Some(path); } diff --git a/src/operation_context.rs b/src/operation_context.rs index d2a443df3..2efd4280b 100644 --- a/src/operation_context.rs +++ b/src/operation_context.rs @@ -65,7 +65,7 @@ pub fn empty() -> OperationContext<'static> { // Return an operation context that contains only global variables, no parser, and never // cancels. pub fn globals() -> OperationContext<'static> { - OperationContext::background(EnvStack::globals(), EXPANSION_LIMIT_DEFAULT) + OperationContext::background(&**EnvStack::globals(), EXPANSION_LIMIT_DEFAULT) } /// Construct from a full set of properties. diff --git a/src/parser.rs b/src/parser.rs index 7840e2cf1..4c343e85d 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -402,13 +402,18 @@ pub fn is_command_substitution(&self) -> bool { pub fn principal_parser() -> &'static Parser { use std::cell::OnceCell; static PRINCIPAL: MainThread> = MainThread::new(OnceCell::new()); - &PRINCIPAL.get().get_or_init(|| { - // Safety: EnvStack::principal() returns a `static` variable guaranteed to always be - // alive. We just don't want to hard-code the lifetime in `Parser` so we wrap the - // `EnvStack` in an `Rc` before sending it along. - let env_rc = unsafe { Rc::from_raw(EnvStack::principal() as *const _) }; - Parser::new(env_rc, true) - }) + &PRINCIPAL + .get() + // The parser is !Send/!Sync and strictly single-threaded, but we can have + // multi-threaded access to its variables stack (why, though?) so EnvStack::principal() + // returns an Arc instead of an Rc. Since the Arc is + // statically allocated and always valid (not even destroyed on exit), we can safely + // transform the Arc into an Rc and save Parser from needing atomic ref counting + // to manage its further references. + .get_or_init(|| { + let env_rc = unsafe { Rc::from_raw(&**EnvStack::principal() as *const _) }; + Parser::new(env_rc, true) + }) } /// Assert that this parser is allowed to execute on the current thread.