From 90fde1a9cda2a0b683fa342b1b4345dd0a88c164 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Mon, 1 Jan 2024 12:34:24 -0800 Subject: [PATCH] Snapshot env when adding to history [ja: this seems a little bit safer?] --- fish-rust/src/env/environment.rs | 16 ---------------- fish-rust/src/history.rs | 8 ++++---- fish-rust/src/reader.rs | 2 +- fish-rust/src/tests/history.rs | 27 +++++++++++++-------------- 4 files changed, 18 insertions(+), 35 deletions(-) diff --git a/fish-rust/src/env/environment.rs b/fish-rust/src/env/environment.rs index 7b33dee93..709e8ceeb 100644 --- a/fish-rust/src/env/environment.rs +++ b/fish-rust/src/env/environment.rs @@ -116,16 +116,6 @@ pub struct EnvDyn { inner: Box, } -pub trait AsEnvironment { - fn as_environment(&self) -> &(dyn Environment + Send + Sync); -} - -impl AsEnvironment for EnvDyn { - fn as_environment(&self) -> &(dyn Environment + Send + Sync) { - &*self.inner - } -} - impl EnvDyn { // Exposed for testing. pub fn new(inner: Box) -> Self { @@ -402,12 +392,6 @@ fn get_pwd_slash(&self) -> WString { // TODO Remove Pin? pub type EnvStackRef = Pin>; -impl AsEnvironment for EnvStackRef { - fn as_environment(&self) -> &(dyn Environment + Send + Sync) { - Pin::get_ref(Pin::as_ref(self)) - } -} - // A variable stack that only represents globals. // Do not push or pop from this. lazy_static! { diff --git a/fish-rust/src/history.rs b/fish-rust/src/history.rs index 7bf139a41..3f351e60c 100644 --- a/fish-rust/src/history.rs +++ b/fish-rust/src/history.rs @@ -46,7 +46,7 @@ str2wcstring, unescape_string, valid_var_name, wcs2zstring, write_loop, CancelChecker, UnescapeStringStyle, }, - env::{AsEnvironment, EnvMode, EnvStack, EnvStackRefFFI, Environment}, + env::{EnvMode, EnvStack, EnvStackRefFFI, Environment}, expand::{expand_one, ExpandFlags}, fallback::fish_mkstemp_cloexec, fds::{wopen_cloexec, AutoCloseFd}, @@ -1688,7 +1688,7 @@ pub fn remove_ephemeral_items(&self) { pub fn add_pending_with_file_detection( self: Arc, s: &wstr, - vars: impl AsEnvironment + Send + Sync + 'static, + vars: &EnvStack, persist_mode: PersistenceMode, /*=disk*/ ) { // We use empty items as sentinels to indicate the end of history. @@ -1745,10 +1745,10 @@ pub fn add_pending_with_file_detection( // Don't hold the lock while we perform this file detection. imp.add(item, /*pending=*/ true, /*do_save=*/ true); drop(imp); + let vars_snapshot = vars.snapshot(); iothread_perform(move || { // Don't hold the lock while we perform this file detection. - let validated_paths = - expand_and_detect_paths(potential_paths, vars.as_environment()); + let validated_paths = expand_and_detect_paths(potential_paths, &vars_snapshot); let mut imp = self.imp(); imp.set_valid_file_paths(validated_paths, identifier); imp.enable_automatic_saving(); diff --git a/fish-rust/src/reader.rs b/fish-rust/src/reader.rs index 55664e08f..cf2a6c6dc 100644 --- a/fish-rust/src/reader.rs +++ b/fish-rust/src/reader.rs @@ -4714,7 +4714,7 @@ fn add_to_history(&mut self) { }; self.history.clone().add_pending_with_file_detection( &text, - self.parser().variables.clone(), + &self.parser().variables, mode, ); } diff --git a/fish-rust/src/tests/history.rs b/fish-rust/src/tests/history.rs index e083c389a..e67773564 100644 --- a/fish-rust/src/tests/history.rs +++ b/fish-rust/src/tests/history.rs @@ -1,7 +1,7 @@ use crate::common::{ cstr2wcstring, is_windows_subsystem_for_linux, str2wcstring, wcs2osstring, wcs2string, }; -use crate::env::{EnvDyn, Environment}; +use crate::env::{EnvDyn, EnvMode, EnvStack, Environment}; use crate::fds::{wopen_cloexec, AutoCloseFd}; use crate::ffi_tests::add_test; use crate::history::{self, History, HistoryItem, HistorySearch, PathList, SearchDirection}; @@ -453,57 +453,56 @@ fn test_history_races_pound_on_history(item_count: usize, idx: usize) { let filename = L!("testfile"); std::fs::write(wcs2osstring(&(tmpdir.clone() + &filename[..])), []).unwrap(); - let mut test_vars = TestEnvironment::default(); - test_vars.vars.insert(L!("PWD").to_owned(), tmpdir.clone()); - test_vars.vars.insert(L!("HOME").to_owned(), tmpdir.clone()); - let vars = || EnvDyn::new(Box::new(test_vars.clone()) as Box); + let test_vars = EnvStack::new(); + test_vars.set_one(L!("PWD"), EnvMode::GLOBAL, tmpdir.clone()); + test_vars.set_one(L!("HOME"), EnvMode::GLOBAL, tmpdir.clone()); let history = History::with_name(L!("path_detection")); history.clear(); assert_eq!(history.size(), 0); history.clone().add_pending_with_file_detection( L!("cmd0 not/a/valid/path"), - vars(), + &test_vars, history::PersistenceMode::Disk, ); history.clone().add_pending_with_file_detection( &(L!("cmd1 ").to_owned() + filename), - vars(), + &test_vars, history::PersistenceMode::Disk, ); history.clone().add_pending_with_file_detection( &(L!("cmd2 ").to_owned() + &tmpdir[..] + L!("/") + filename), - vars(), + &test_vars, history::PersistenceMode::Disk, ); history.clone().add_pending_with_file_detection( &(L!("cmd3 $HOME/").to_owned() + filename), - vars(), + &test_vars, history::PersistenceMode::Disk, ); history.clone().add_pending_with_file_detection( L!("cmd4 $HOME/notafile"), - vars(), + &test_vars, history::PersistenceMode::Disk, ); history.clone().add_pending_with_file_detection( &(L!("cmd5 ~/").to_owned() + filename), - vars(), + &test_vars, history::PersistenceMode::Disk, ); history.clone().add_pending_with_file_detection( L!("cmd6 ~/notafile"), - vars(), + &test_vars, history::PersistenceMode::Disk, ); history.clone().add_pending_with_file_detection( L!("cmd7 ~/*f*"), - vars(), + &test_vars, history::PersistenceMode::Disk, ); history.clone().add_pending_with_file_detection( L!("cmd8 ~/*zzz*"), - vars(), + &test_vars, history::PersistenceMode::Disk, ); history.resolve_pending();