Fix a deadlock affecting fish_config

This fixes the following deadlock. The C++ functions path_get_config and
path_get_data lazily determine paths and then cache those in a C++ static
variable. The path determination requires inspecting the environment stack.
If these functions are first called while the environment stack is locked
(in this case, when fetching the $history variable) we can get a deadlock.

The fix is to call them eagerly during env_init. This can be removed once
the corresponding C++ functions are removed.

This issue caused fish_config to fail to report colors and themes.

Add a test.
This commit is contained in:
ridiculousfish 2023-10-07 15:20:09 -07:00
parent b315b66cb0
commit f7e7396c69
3 changed files with 26 additions and 0 deletions

View file

@ -520,6 +520,10 @@ fn setup_path() {
/// This is a simple key->value map and not e.g. cut into paths. /// This is a simple key->value map and not e.g. cut into paths.
pub static INHERITED_VARS: OnceCell<HashMap<WString, WString>> = OnceCell::new(); pub static INHERITED_VARS: OnceCell<HashMap<WString, WString>> = OnceCell::new();
extern "C" {
fn env_cpp_init();
}
pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool) { pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool) {
let vars = &PRINCIPAL_STACK; let vars = &PRINCIPAL_STACK;
@ -586,6 +590,11 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool
} }
} }
// See the env_cpp_init() comment for this.
unsafe {
env_cpp_init();
}
// Set $USER, $HOME and $EUID // Set $USER, $HOME and $EUID
// This involves going to passwd and stuff. // This involves going to passwd and stuff.
vars.set_one( vars.set_one(

View file

@ -148,6 +148,20 @@ void misc_init() {
} }
} }
extern "C" {
void env_cpp_init() {
// Temporary for the Rust port.
// path_get_config and path_get_data both inspect the environment stack to determine
// config paths, and then remember those paths in a static variable. This can lead to
// a deadlock if these functions are called while already holding the environment lock.
// Call them immediately to trigger the caching.
// This can be removed once path_get_config and path_get_data are removed.
wcstring dir;
path_get_config(dir);
path_get_data(dir);
}
}
static std::map<wcstring, wcstring> inheriteds; static std::map<wcstring, wcstring> inheriteds;
const std::map<wcstring, wcstring> &env_get_inherited() { return inheriteds; } const std::map<wcstring, wcstring> &env_get_inherited() { return inheriteds; }

View file

@ -105,3 +105,6 @@ $fish --no-config -c 'echo notprinted | and true'
# CHECKERR: echo notprinted | and true # CHECKERR: echo notprinted | and true
# CHECKERR: ^~^ # CHECKERR: ^~^
# Regression test for a hang.
echo "set -L" | $fish > /dev/null