Simplify Parser and EnvStack singletons and clarify thread semantics

`Parser` is a single-threaded `!Send`, `!Sync` type and does not need to use
`Arc` for anything. We were using it because that's all we had for the parser's
`EnvStack`, but though that is *technically* protected internally by a mutex
(shared with global EnvStack), there's nothing to say that other parsers with a
narrower scope/lifetime on other threads will be necessarily using the same
backing mutex.

We can safely marshal the existing `Arc<EnvStack>` we get from
`environment::principal()` into an `Rc<EnvStack>` since the underlying reference
is always valid. To prove this point, we could have PRINCIPAL_STACK be a static
`EnvStack` and have `environment::principal()` use `Arc::from_raw()` to turn
that into an `Arc<EnvStack>`, but there's no need to factorize this process.

src/bin/fish_indent.rs

@@ -990,11 +990,7 @@ fn throwing_main() -> i32 {
                 }
             }
             OutputType::Ansi => {
-                colored_output = colorize(
+                colored_output = colorize(&output_wtext, &colors, &**EnvStack::globals());
-                    &output_wtext,
-                    &colors,
-                    EnvStack::globals().as_ref().get_ref(),
-                );
             }
             OutputType::Html => {
                 colored_output = html_colorize(&output_wtext, &colors);

src/env/environment.rs

@@ -27,7 +27,6 @@ use crate::wcstringutil::join_strings;
 use crate::wutil::{fish_wcstol, wgetcwd, wgettext};
 use std::sync::atomic::Ordering;
 
-use lazy_static::lazy_static;
 use libc::{c_int, uid_t, STDOUT_FILENO, _IONBF};
 use once_cell::sync::OnceCell;
 use std::collections::HashMap;
@@ -35,7 +34,6 @@ use std::ffi::CStr;
 use std::io::Write;
 use std::mem::MaybeUninit;
 use std::os::unix::prelude::*;
-use std::pin::Pin;
 use std::sync::Arc;
 
 /// Set when a universal variable has been modified but not yet been written to disk via sync().
@@ -190,7 +188,7 @@ impl EnvStack {
 
     /// Return whether we are the principal stack.
     pub fn is_principal(&self) -> bool {
-        std::ptr::eq(self, Self::principal().as_ref().get_ref())
+        std::ptr::eq(self, &**Self::principal())
     }
 
     /// Helpers to get and set the proc statuses.
@@ -362,13 +360,17 @@ impl EnvStack {
 
     /// A variable stack that only represents globals.
     /// Do not push or pop from this.
-    pub fn globals() -> &'static EnvStackRef {
+    pub fn globals() -> &'static Arc<EnvStack> {
-        &GLOBALS
+        use std::sync::OnceLock;
+        static GLOBALS: OnceLock<Arc<EnvStack>> = OnceLock::new();
+        &GLOBALS.get_or_init(|| Arc::new(EnvStack::new()))
     }
 
     /// Access the principal variable stack, associated with the principal parser.
-    pub fn principal() -> &'static EnvStackRef {
+    pub fn principal() -> &'static Arc<EnvStack> {
-        &PRINCIPAL_STACK
+        use std::sync::OnceLock;
+        static PRINCIPAL_STACK: OnceLock<Arc<EnvStack>> = OnceLock::new();
+        &PRINCIPAL_STACK.get_or_init(|| Arc::new(EnvStack::new()))
     }
 
     pub fn set_argv(&self, argv: Vec<WString>) {
@@ -408,20 +410,6 @@ impl Environment for EnvStack {
     }
 }
 
-// TODO Remove Pin?
-pub type EnvStackRef = Pin<Arc<EnvStack>>;
-
-// A variable stack that only represents globals.
-// Do not push or pop from this.
-lazy_static! {
-    static ref GLOBALS: EnvStackRef = Arc::pin(EnvStack::new());
-}
-
-// Our singleton "principal" stack.
-lazy_static! {
-    static ref PRINCIPAL_STACK: EnvStackRef = Arc::pin(EnvStack::new());
-}
-
 /// Some configuration path environment variables.
 const FISH_DATADIR_VAR: &wstr = L!("__fish_data_dir");
 const FISH_SYSCONFDIR_VAR: &wstr = L!("__fish_sysconf_dir");
@@ -539,7 +527,7 @@ fn setup_user(vars: &EnvStack) {
 fn setup_path() {
     use crate::libc::{confstr, _CS_PATH};
 
-    let vars = &GLOBALS;
+    let vars = EnvStack::globals();
     let path = vars.get_unless_empty(L!("PATH"));
     if path.is_none() {
         // _CS_PATH: colon-separated paths to find POSIX utilities
@@ -566,7 +554,7 @@ fn setup_path() {
 pub static INHERITED_VARS: OnceCell<HashMap<WString, WString>> = OnceCell::new();
 
 pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool) {
-    let vars = &PRINCIPAL_STACK;
+    let vars = &**EnvStack::principal();
 
     let env_iter: Vec<_> = std::env::vars_os()
         .map(|(k, v)| (str2wcstring(k.as_bytes()), str2wcstring(v.as_bytes())))
@@ -725,7 +713,7 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool
 
     // Initialize termsize variables.
     // PORTING: 3x deref is weird
-    let termsize = termsize::SHARED_CONTAINER.initialize(&***vars as &dyn Environment);
+    let termsize = termsize::SHARED_CONTAINER.initialize(vars as &dyn Environment);
     if vars.get_unless_empty(L!("COLUMNS")).is_none() {
         vars.set_one(L!("COLUMNS"), EnvMode::GLOBAL, termsize.width.to_wstring());
     }

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().as_ref().get_ref())
+                .resolve_command(name, &**EnvStack::globals())
             {
                 path_to_autoload = Some(path);
             }

src/operation_context.rs

@@ -1,10 +1,8 @@
 use crate::common::CancelChecker;
 use crate::env::EnvDyn;
-use crate::env::{EnvStack, EnvStackRef, Environment};
+use crate::env::{EnvStack, Environment};
 use crate::parser::{Parser, ParserRef};
 use crate::proc::JobGroupRef;
-use once_cell::sync::Lazy;
-use std::sync::Arc;
 
 use crate::reader::read_generation_count;
 
@@ -47,8 +45,6 @@ pub struct OperationContext<'a> {
     pub cancel_checker: CancelChecker,
 }
 
-static nullenv: Lazy<EnvStackRef> = Lazy::new(|| Arc::pin(EnvStack::new()));
-
 impl<'a> OperationContext<'a> {
     pub fn vars(&self) -> &dyn Environment {
         match &self.vars {
@@ -60,7 +56,10 @@ impl<'a> OperationContext<'a> {
 
     // Return an "empty" context which contains no variables, no parser, and never cancels.
     pub fn empty() -> OperationContext<'static> {
-        OperationContext::background(&**nullenv, EXPANSION_LIMIT_DEFAULT)
+        use std::sync::OnceLock;
+        static NULL_ENV: OnceLock<EnvStack> = OnceLock::new();
+        let null_env = NULL_ENV.get_or_init(|| EnvStack::new());
+        OperationContext::background(null_env, EXPANSION_LIMIT_DEFAULT)
     }
 
     // Return an operation context that contains only global variables, no parser, and never

src/parser.rs

@@ -7,7 +7,7 @@ use crate::common::{
     FilenameRef, ScopeGuarding, PROFILING_ACTIVE,
 };
 use crate::complete::CompletionList;
-use crate::env::{EnvMode, EnvStack, EnvStackRef, EnvStackSetResult, Environment, Statuses};
+use crate::env::{EnvMode, EnvStack, EnvStackSetResult, Environment, Statuses};
 use crate::event::{self, Event};
 use crate::expand::{
     expand_string, replace_home_directory_with_tilde, ExpandFlags, ExpandResultCode,
@@ -37,7 +37,6 @@ use std::cell::{Ref, RefCell, RefMut};
 use std::ffi::{CStr, OsStr};
 use std::os::fd::{AsRawFd, OwnedFd, RawFd};
 use std::os::unix::prelude::OsStrExt;
-use std::pin::Pin;
 use std::rc::{Rc, Weak};
 use std::sync::{
     atomic::{AtomicIsize, AtomicU64, Ordering},
@@ -312,7 +311,7 @@ pub struct Parser {
     pub eval_level: AtomicIsize,
 
     /// Set of variables for the parser.
-    pub variables: EnvStackRef,
+    pub variables: Rc<EnvStack>,
 
     /// Miscellaneous library data.
     library_data: RefCell<LibraryData>,
@@ -333,7 +332,7 @@ pub struct Parser {
 
 impl Parser {
     /// Create a parser
-    pub fn new(variables: EnvStackRef, is_principal: bool) -> ParserRef {
+    pub fn new(variables: Rc<EnvStack>, is_principal: bool) -> ParserRef {
         let result = Rc::new_cyclic(|this: &Weak<Self>| Self {
             this: Weak::clone(this),
             execution_context: RefCell::default(),
@@ -405,7 +404,16 @@ impl Parser {
         static PRINCIPAL: MainThread<OnceCell<ParserRef>> = MainThread::new(OnceCell::new());
         &PRINCIPAL
             .get()
-            .get_or_init(|| Parser::new(EnvStack::principal().clone(), true))
+            // 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<EnvStack> instead of an Rc<EnvStack>. Since the Arc<EnvStack> is
+            // statically allocated and always valid (not even destroyed on exit), we can safely
+            // transform the Arc<T> into an Rc<T> 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.
@@ -753,8 +761,8 @@ impl Parser {
     }
 
     /// Get the variables as an Arc.
-    pub fn vars_ref(&self) -> Arc<EnvStack> {
+    pub fn vars_ref(&self) -> Rc<EnvStack> {
-        Pin::into_inner(Pin::clone(&self.variables))
+        Rc::clone(&self.variables)
     }
 
     /// Get the library data.

src/tests/input.rs

@@ -4,14 +4,14 @@ use crate::key::Key;
 use crate::parser::Parser;
 use crate::tests::prelude::*;
 use crate::wchar::prelude::*;
-use std::sync::Arc;
+use std::rc::Rc;
 
 #[test]
 #[serial]
 fn test_input() {
     let _cleanup = test_init();
     use crate::env::EnvStack;
-    let parser = Parser::new(Arc::pin(EnvStack::new()), false);
+    let parser = Parser::new(Rc::new(EnvStack::new()), false);
     let mut input = Inputter::new(parser, libc::STDIN_FILENO);
     // Ensure sequences are order independent. Here we add two bindings where the first is a prefix
     // of the second, and then emit the second key list. The second binding should be invoked, not