h/fish-shell
1
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2026-06-09 03:51:20 -03:00
Code Issues Packages Projects Releases Wiki Activity

Use shared file locking logic

Browse Source 
Both history files and universal variables files are accessed by multiple
processes, which need a way to synchronize their accesses.
This synchronization logic is mixed in with the logic for reading and updating
the files' contents, which results in messy code, duplicated locking
implementations, and inconsistencies.
Moreover, the existing implementations are flawed which has resulted in file
corruption (e.g. https://github.com/fish-shell/fish-shell/issues/10300).

The new approach separates the synchronization logic from the rest.

There are two approaches to synchronization.
- The primary one is using `flock(2)` to lock the directory containing the file
  which requires synchronized access. We do not lock the file holding the data
  directly because this file might be replaced, which can result in locking
  succeeding when it should block. Locking the directory solves this problem.
  To avoid inconsistent file states, changes are first written to a temporary
  file, which is then renamed into place while still holding the lock.
- In some situations `flock` locks are unavailable or take a long time. This
  mostly applies to remote file systems. If we think that a directory is located
  on a remote file system, we do not attempt to use `flock`.
  As a fallback, we have a lockless approach, which uses file metadata (device,
  inode, size, ctime, mtime) to identify file versions.
  We then read from the file, write the updated data to a temporary file,
  check if the file id for the path is the same as before we started reading,
  and if so we rename the temporary file such that it replaces the old file.
  Note that races are possible between the file id check and the rename syscall.
  If we detect a file id mismatch, we retry, up to a predetermined number of
  attempts.

The operations which should be performed are passed to the functions handling
synchronization as `Fn`s. Because we might have to run these operations
repeatedly when retrying, they should be executable arbitrarily often without
causing side-effects relevant to the program. This requires some changes to
functions accessing these files. In many cases, they have to work with
non-mutable references, which requires that they return the data which should be
updated in the program state, instead of directly assigning to the appropriate
location.

Locking via `O_EXLOCK`, which was used for the universal variable file, is no
longer supported. That version of locking locks the file operated on directly,
instead of its directory. According to the man pages of {Open,Free,Net}BSD
and macOS, these locks have flock semantics. So if flock is available, we can
use it, and if it is not, the `O_EXLOCK` flag does not help.
This commit is contained in:
Daniel Rainer
2025-06-15 18:21:02 +02:00
parent ccb9c8225f
commit f438e80f9b
14 changed files with 738 additions and 836 deletions
Download Patch File Download Diff File Expand all files Collapse all files

52
po/de.po
View File

@@ -1025,8 +1025,8 @@ msgid "Error reading script file '%s':"
msgstr "Die Script-Datei '%s' kann nicht gelesen werden:"
#, c-format
msgid "Error when renaming history file: %s"
msgstr "Fehler beim Umbenennen der Verlaufsdatei: %s"
msgid "Error when renaming file: %s"
msgstr ""
#, c-format
msgid "Error while reading file %ls\n"
@@ -1249,14 +1249,6 @@ msgstr ""
msgid "List or remove functions"
msgstr "Funktionen auflisten oder entfernen"
#, c-format
msgid "Locking the history file took too long (%.3f seconds)."
msgstr "Sperren der Geschichts-Datei dauerte zu lang (%.3f Sekunden)."
#, c-format
msgid "Locking the universal var file took too long (%.3f seconds)."
msgstr "Die Datei für universelle Variablen zu sperren dauerte zu lange (%.3f Sekunden)"
msgid "Logical operations are not supported, use `test` instead"
msgstr "Logikoperationen werden nicht unterstützt, nimm stattdessen `test`"
@@ -1519,6 +1511,10 @@ msgstr "Derzeit ausgewertete Funktion stoppen"
msgid "Stop the innermost loop"
msgstr "Innerste Schleife beenden"
#
msgid "Synchronized file access"
msgstr ""
msgid "Temporarily block delivery of events"
msgstr "Ereignisweitergabe vorübergehend blockieren"
@@ -1623,22 +1619,10 @@ msgstr "Konnte Verzeichnis %ls abgeleitet von %ls nicht finden: '%ls'."
msgid "Unable to locate the %ls directory."
msgstr ""
#, c-format
msgid "Unable to open universal variable file '%s': %s"
msgstr "Die Universal-Variablen-Datei '%s' kann nicht geöffnet werden: %s"
#, c-format
msgid "Unable to read input file: %s"
msgstr ""
#, c-format
msgid "Unable to rename file from '%ls' to '%ls': %s"
msgstr "Konnte Datei '%ls' nicht zu '%ls' umbenennen: %s"
#, c-format
msgid "Unable to write to universal variables file '%ls': %s"
msgstr "Die Universal-Variablen-Datei '%ls' kann nicht geschrieben werden: %s"
msgid "Unexpected ')' for unopened parenthesis"
msgstr ""
@@ -79192,6 +79176,30 @@ msgstr ""
msgid "~ expansion"
msgstr ""
#, c-format
#~ msgid "Error when renaming history file: %s"
#~ msgstr "Fehler beim Umbenennen der Verlaufsdatei: %s"
#, c-format
#~ msgid "Locking the history file took too long (%.3f seconds)."
#~ msgstr "Sperren der Geschichts-Datei dauerte zu lang (%.3f Sekunden)."
#, c-format
#~ msgid "Locking the universal var file took too long (%.3f seconds)."
#~ msgstr "Die Datei für universelle Variablen zu sperren dauerte zu lange (%.3f Sekunden)"
#, c-format
#~ msgid "Unable to open universal variable file '%s': %s"
#~ msgstr "Die Universal-Variablen-Datei '%s' kann nicht geöffnet werden: %s"
#, c-format
#~ msgid "Unable to rename file from '%ls' to '%ls': %s"
#~ msgstr "Konnte Datei '%ls' nicht zu '%ls' umbenennen: %s"
#, c-format
#~ msgid "Unable to write to universal variables file '%ls': %s"
#~ msgstr "Die Universal-Variablen-Datei '%ls' kann nicht geschrieben werden: %s"
#~ msgid "builtin\n"
#~ msgstr "Eingebauter Befehl\n"

30
po/en.po
View File

@@ -1023,7 +1023,7 @@ msgid "Error reading script file '%s':"
msgstr ""
#, c-format
msgid "Error when renaming history file: %s"
msgid "Error when renaming file: %s"
msgstr ""
#, c-format
@@ -1246,14 +1246,6 @@ msgstr ""
msgid "List or remove functions"
msgstr "List or remove functions"
#, c-format
msgid "Locking the history file took too long (%.3f seconds)."
msgstr ""
#, c-format
msgid "Locking the universal var file took too long (%.3f seconds)."
msgstr ""
msgid "Logical operations are not supported, use `test` instead"
msgstr ""
@@ -1516,6 +1508,10 @@ msgstr "Stop the currently evaluated function"
msgid "Stop the innermost loop"
msgstr "Stop the innermost loop"
#
msgid "Synchronized file access"
msgstr ""
msgid "Temporarily block delivery of events"
msgstr "Temporarily block delivery of events"
@@ -1620,22 +1616,10 @@ msgstr ""
msgid "Unable to locate the %ls directory."
msgstr "Unable to locate the %ls directory."
#, c-format
msgid "Unable to open universal variable file '%s': %s"
msgstr ""
#, c-format
msgid "Unable to read input file: %s"
msgstr ""
#, c-format
msgid "Unable to rename file from '%ls' to '%ls': %s"
msgstr ""
#, c-format
msgid "Unable to write to universal variables file '%ls': %s"
msgstr "Unable to write to universal variables file '%ls': %s"
msgid "Unexpected ')' for unopened parenthesis"
msgstr ""
@@ -79188,6 +79172,10 @@ msgstr ""
msgid "~ expansion"
msgstr ""
#, c-format
#~ msgid "Unable to write to universal variables file '%ls': %s"
#~ msgstr "Unable to write to universal variables file '%ls': %s"
#~ msgid "empty"
#~ msgstr "empty"

42
po/fr.po
View File

@@ -1124,7 +1124,7 @@ msgid "Error reading script file '%s':"
msgstr ""
#, c-format
msgid "Error when renaming history file: %s"
msgid "Error when renaming file: %s"
msgstr ""
#, c-format
@@ -1347,14 +1347,6 @@ msgstr ""
msgid "List or remove functions"
msgstr "Lister ou supprimer des fonctions"
#, c-format
msgid "Locking the history file took too long (%.3f seconds)."
msgstr "Le verrouillage du fichier d’historique a pris trop de temps (%.3f secondes)."
#, c-format
msgid "Locking the universal var file took too long (%.3f seconds)."
msgstr "Le verrouillage du fichier des variables universel a pris trop de temps (%.3f secondes)."
msgid "Logical operations are not supported, use `test` instead"
msgstr ""
@@ -1617,6 +1609,10 @@ msgstr "Arrêter la fonction actuellement en évaluation"
msgid "Stop the innermost loop"
msgstr "Arrêter la boucle interne"
#
msgid "Synchronized file access"
msgstr ""
msgid "Temporarily block delivery of events"
msgstr "Bloquer temporairement la distribution des événements"
@@ -1721,22 +1717,10 @@ msgstr "Impossible de localiser le dossier %ls obtenu de $%ls: '%ls'."
msgid "Unable to locate the %ls directory."
msgstr "Impossible de localiser le dossier %ls."
#, c-format
msgid "Unable to open universal variable file '%s': %s"
msgstr ""
#, c-format
msgid "Unable to read input file: %s"
msgstr ""
#, c-format
msgid "Unable to rename file from '%ls' to '%ls': %s"
msgstr "Impossible de renommer le fichier '%ls' en '%ls': %s"
#, c-format
msgid "Unable to write to universal variables file '%ls': %s"
msgstr "Impossible d’écrire dans le fichier de variables universelles '%ls': %s"
msgid "Unexpected ')' for unopened parenthesis"
msgstr ""
@@ -79289,6 +79273,22 @@ msgstr ""
msgid "~ expansion"
msgstr ""
#, c-format
#~ msgid "Locking the history file took too long (%.3f seconds)."
#~ msgstr "Le verrouillage du fichier d’historique a pris trop de temps (%.3f secondes)."
#, c-format
#~ msgid "Locking the universal var file took too long (%.3f seconds)."
#~ msgstr "Le verrouillage du fichier des variables universel a pris trop de temps (%.3f secondes)."
#, c-format
#~ msgid "Unable to rename file from '%ls' to '%ls': %s"
#~ msgstr "Impossible de renommer le fichier '%ls' en '%ls': %s"
#, c-format
#~ msgid "Unable to write to universal variables file '%ls': %s"
#~ msgstr "Impossible d’écrire dans le fichier de variables universelles '%ls': %s"
#~ msgid "Check that this terminal type is supported on this system."
#~ msgstr "Vérifiez que votre type de terminal est supporté sur ce système"

26
po/pl.po
View File

@@ -1019,7 +1019,7 @@ msgid "Error reading script file '%s':"
msgstr ""
#, c-format
msgid "Error when renaming history file: %s"
msgid "Error when renaming file: %s"
msgstr ""
#, c-format
@@ -1242,14 +1242,6 @@ msgstr ""
msgid "List or remove functions"
msgstr "Wypisz lub usuń funkcje"
#, c-format
msgid "Locking the history file took too long (%.3f seconds)."
msgstr ""
#, c-format
msgid "Locking the universal var file took too long (%.3f seconds)."
msgstr ""
msgid "Logical operations are not supported, use `test` instead"
msgstr ""
@@ -1512,6 +1504,10 @@ msgstr "Zatrzymaj obecnie używaną funkcję"
msgid "Stop the innermost loop"
msgstr ""
#
msgid "Synchronized file access"
msgstr ""
msgid "Temporarily block delivery of events"
msgstr ""
@@ -1616,22 +1612,10 @@ msgstr ""
msgid "Unable to locate the %ls directory."
msgstr ""
#, c-format
msgid "Unable to open universal variable file '%s': %s"
msgstr ""
#, c-format
msgid "Unable to read input file: %s"
msgstr ""
#, c-format
msgid "Unable to rename file from '%ls' to '%ls': %s"
msgstr ""
#, c-format
msgid "Unable to write to universal variables file '%ls': %s"
msgstr ""
msgid "Unexpected ')' for unopened parenthesis"
msgstr ""

26
po/pt_BR.po
View File

@@ -1024,7 +1024,7 @@ msgid "Error reading script file '%s':"
msgstr ""
#, c-format
msgid "Error when renaming history file: %s"
msgid "Error when renaming file: %s"
msgstr ""
#, c-format
@@ -1247,14 +1247,6 @@ msgstr ""
msgid "List or remove functions"
msgstr "Lista ou remove funções"
#, c-format
msgid "Locking the history file took too long (%.3f seconds)."
msgstr ""
#, c-format
msgid "Locking the universal var file took too long (%.3f seconds)."
msgstr ""
msgid "Logical operations are not supported, use `test` instead"
msgstr ""
@@ -1517,6 +1509,10 @@ msgstr "Pára a função em execução"
msgid "Stop the innermost loop"
msgstr "Pára o laço mais interno"
#
msgid "Synchronized file access"
msgstr ""
msgid "Temporarily block delivery of events"
msgstr "Bloqueia temporariamente a entrega de eventos"
@@ -1621,22 +1617,10 @@ msgstr ""
msgid "Unable to locate the %ls directory."
msgstr ""
#, c-format
msgid "Unable to open universal variable file '%s': %s"
msgstr ""
#, c-format
msgid "Unable to read input file: %s"
msgstr ""
#, c-format
msgid "Unable to rename file from '%ls' to '%ls': %s"
msgstr ""
#, c-format
msgid "Unable to write to universal variables file '%ls': %s"
msgstr ""
msgid "Unexpected ')' for unopened parenthesis"
msgstr ""

26
po/sv.po
View File

@@ -1020,7 +1020,7 @@ msgid "Error reading script file '%s':"
msgstr ""
#, c-format
msgid "Error when renaming history file: %s"
msgid "Error when renaming file: %s"
msgstr ""
#, c-format
@@ -1243,14 +1243,6 @@ msgstr ""
msgid "List or remove functions"
msgstr "Visa eller ta bort funktioner"
#, c-format
msgid "Locking the history file took too long (%.3f seconds)."
msgstr ""
#, c-format
msgid "Locking the universal var file took too long (%.3f seconds)."
msgstr ""
msgid "Logical operations are not supported, use `test` instead"
msgstr ""
@@ -1513,6 +1505,10 @@ msgstr "Avbryt den nuvarande funktionen"
msgid "Stop the innermost loop"
msgstr "Avbryt den innersta loopen"
#
msgid "Synchronized file access"
msgstr ""
msgid "Temporarily block delivery of events"
msgstr "Blockera tillfälligt leverans av händelser"
@@ -1617,22 +1613,10 @@ msgstr ""
msgid "Unable to locate the %ls directory."
msgstr ""
#, c-format
msgid "Unable to open universal variable file '%s': %s"
msgstr ""
#, c-format
msgid "Unable to read input file: %s"
msgstr ""
#, c-format
msgid "Unable to rename file from '%ls' to '%ls': %s"
msgstr ""
#, c-format
msgid "Unable to write to universal variables file '%ls': %s"
msgstr ""
msgid "Unexpected ')' for unopened parenthesis"
msgstr ""

52
po/zh_CN.po
View File

@@ -1017,8 +1017,8 @@ msgid "Error reading script file '%s':"
msgstr "读取脚本文件 '%s'出错: "
#, c-format
msgid "Error when renaming history file: %s"
msgstr "重命名历史文件时出错:%s"
msgid "Error when renaming file: %s"
msgstr ""
#, c-format
msgid "Error while reading file %ls\n"
@@ -1241,14 +1241,6 @@ msgstr ""
msgid "List or remove functions"
msgstr "列出或移除函数"
#, c-format
msgid "Locking the history file took too long (%.3f seconds)."
msgstr "锁定历史文件花费了太长时间(%.3f 秒)."
#, c-format
msgid "Locking the universal var file took too long (%.3f seconds)."
msgstr "锁定 通用 var 文件花费了太长时间 (%.3f 秒) ."
msgid "Logical operations are not supported, use `test` instead"
msgstr "不支持逻辑操作,使用`test`替代"
@@ -1511,6 +1503,10 @@ msgstr "停止当前求值的函数"
msgid "Stop the innermost loop"
msgstr "停止最内层的循环"
#
msgid "Synchronized file access"
msgstr ""
msgid "Temporarily block delivery of events"
msgstr "Temporarily block delivery of events"
@@ -1615,22 +1611,10 @@ msgstr "无法定位来自 $ls 的 %ls 目录: %ls ."
msgid "Unable to locate the %ls directory."
msgstr "无法找到 %ls 目录 ."
#, c-format
msgid "Unable to open universal variable file '%s': %s"
msgstr "无法打开通用变量文件'%s': %s"
#, c-format
msgid "Unable to read input file: %s"
msgstr "无法读取输入文件:%s"
#, c-format
msgid "Unable to rename file from '%ls' to '%ls': %s"
msgstr "无法将文件从 '%ls' 重命名为 '%ls':%s"
#, c-format
msgid "Unable to write to universal variables file '%ls': %s"
msgstr "无法写入通用变量文件 '%ls':%s"
msgid "Unexpected ')' for unopened parenthesis"
msgstr "未打开的括号' ' "
@@ -79187,6 +79171,30 @@ msgstr "缩放"
msgid "~ expansion"
msgstr "~ 扩大"
#, c-format
#~ msgid "Error when renaming history file: %s"
#~ msgstr "重命名历史文件时出错:%s"
#, c-format
#~ msgid "Locking the history file took too long (%.3f seconds)."
#~ msgstr "锁定历史文件花费了太长时间(%.3f 秒)."
#, c-format
#~ msgid "Locking the universal var file took too long (%.3f seconds)."
#~ msgstr "锁定 通用 var 文件花费了太长时间 (%.3f 秒) ."
#, c-format
#~ msgid "Unable to open universal variable file '%s': %s"
#~ msgstr "无法打开通用变量文件'%s': %s"
#, c-format
#~ msgid "Unable to rename file from '%ls' to '%ls': %s"
#~ msgstr "无法将文件从 '%ls' 重命名为 '%ls':%s"
#, c-format
#~ msgid "Unable to write to universal variables file '%ls': %s"
#~ msgstr "无法写入通用变量文件 '%ls':%s"
#~ msgid "builtin\n"
#~ msgstr "内建\n"

422
src/env_universal_common.rs
View File

@@ -1,40 +1,28 @@
#![allow(clippy::bad_bit_mask)]
use crate::common::{
timef, unescape_string, valid_var_name, wcs2zstring, UnescapeFlags, UnescapeStringStyle,
unescape_string, valid_var_name, wcs2zstring, UnescapeFlags, UnescapeStringStyle,
};
use crate::env::{EnvVar, EnvVarFlags, VarTable};
use crate::fds::{open_cloexec, wopen_cloexec};
use crate::flog::{FLOG, FLOGF};
use crate::fs::create_temporary_file;
use crate::fs::{lock_and_load, rewrite_via_temporary_file, PotentialUpdate};
use crate::global_safety::RelaxedAtomicBool;
use crate::path::path_get_config;
use crate::path::{path_get_config_remoteness, DirRemoteness};
use crate::wchar::{decode_byte_from_char, prelude::*};
use crate::wcstringutil::{join_strings, LineIterator};
use crate::wutil::{
file_id_for_file, file_id_for_path, file_id_for_path_narrow, wdirname, wrealpath, wrename,
wstat, wunlink, FileId, INVALID_FILE_ID,
};
use errno::{errno, Errno};
use libc::{EINTR, LOCK_EX};
use nix::{fcntl::OFlag, sys::stat::Mode};
use crate::wutil::{file_id_for_file, file_id_for_path_narrow, wrealpath, FileId, INVALID_FILE_ID};
use std::collections::hash_map::Entry;
use std::collections::HashSet;
use std::ffi::CString;
use std::fs::File;
use std::io::{Read, Write};
use std::mem::MaybeUninit;
use std::os::fd::AsRawFd;
use std::os::unix::prelude::MetadataExt;
// Pull in the O_EXLOCK constant if it is defined, otherwise set it to 0.
#[cfg(any(apple, bsd))]
const O_EXLOCK: OFlag = OFlag::O_EXLOCK;
#[cfg(not(any(apple, bsd)))]
const O_EXLOCK: OFlag = OFlag::empty();
static LOCK_UVAR_FILE: RelaxedAtomicBool = RelaxedAtomicBool::new(true);
/// Callback data, reflecting a change in universal variables.
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct CallbackData {
// The name of the variable.
pub key: WString,
@@ -72,13 +60,11 @@ pub struct EnvUniversal {
// Whether it's OK to save. This may be set to false if we discover that a future version of
// fish wrote the uvars contents.
// Only update if last_read_file_id is updated as well.
ok_to_save: bool,
// If true, attempt to flock the uvars file.
// This latches to false if the file is found to be remote, where flock may hang.
do_flock: bool,
// File id from which we last read.
// Only update if ok_to_save is updated as well.
last_read_file_id: FileId,
}
@@ -99,7 +85,6 @@ pub fn new() -> Self {
modified: Default::default(),
export_generation: 1,
ok_to_save: true,
do_flock: true,
last_read_file_id: INVALID_FILE_ID,
}
}
@@ -161,10 +146,10 @@ pub fn get_table(&self) -> &VarTable {
/// Initialize this uvars for the default path.
/// This should be called at most once on any given instance.
pub fn initialize(&mut self) -> Option<CallbackDataList> {
// Set do_flock to false immediately if the default variable path is on a remote filesystem.
// Set `LOCK_UVAR_FILE` to false immediately if the default variable path is on a remote filesystem.
// See #7968.
if path_get_config_remoteness() == DirRemoteness::remote {
self.do_flock = false;
LOCK_UVAR_FILE.store(false);
}
self.initialize_at_path(default_vars_path())
}
@@ -181,43 +166,16 @@ pub fn initialize_at_path(&mut self, path: WString) -> Option<CallbackDataList>
self.load_from_path()
}
/// Reads and writes variables at the correct path. Returns true if modified variables were
/// written.
/// Reads and writes variables at the correct path.
/// If there is an existing variables file with an unknown format, parsing it is attempted,
/// but it will not be overwritten.
/// Returns whether data was read, and the callbacks.
pub fn sync(&mut self) -> (bool, Option<CallbackDataList>) {
if !self.initialized() {
return (false, None);
}
FLOG!(uvar_file, "universal log sync");
// Our saving strategy:
//
// 1. Open the file, producing an fd.
// 2. Lock the file (may be combined with step 1 on systems with O_EXLOCK)
// 3. After taking the lock, check if the file at the given path is different from what we
// opened. If so, start over.
// 4. Read from the file. This can be elided if its dev/inode is unchanged since the last read
// 5. Open an adjacent temporary file
// 6. Write our changes to an adjacent file
// 7. Move the adjacent file into place via rename. This is assumed to be atomic.
// 8. Release the lock and close the file
//
// Consider what happens if Process 1 and 2 both do this simultaneously. Can there be data loss?
// Process 1 opens the file and then attempts to take the lock. Now, either process 1 will see
// the original file, or process 2's new file. If it sees the new file, we're OK: it's going to
// read from the new file, and so there's no data loss. If it sees the old file, then process 2
// must have locked it (if process 1 locks it, switch their roles). The lock will block until
// process 2 reaches step 7; at that point process 1 will reach step 2, notice that the file has
// changed, and then start over.
//
// It's possible that the underlying filesystem does not support locks (lockless NFS). In this
// case, we risk data loss if two shells try to write their universal variables simultaneously.
// In practice this is unlikely, since uvars are usually written interactively.
//
// Prior versions of fish used a hard link scheme to support file locking on lockless NFS. The
// risk here is that if the process crashes or is killed while holding the lock, future
// instances of fish will not be able to obtain it. This seems to be a greater risk than that of
// data loss on lockless NFS. Users who put their home directory on lockless NFS are playing
// with fire anyways.
// If we have no changes, just load.
if self.modified.is_empty() {
let callbacks = self.load_from_path_narrow();
@@ -225,40 +183,60 @@ pub fn sync(&mut self) -> (bool, Option<CallbackDataList>) {
return (false, callbacks);
}
let directory = wdirname(&self.vars_path).to_owned();
FLOG!(uvar_file, "universal log performing full sync");
// Open the file.
let Some(vars_file) = self.open_and_acquire_lock() else {
FLOG!(uvar_file, "universal log open_and_acquire_lock() failed");
return (false, None);
};
// TODO: proper locking
match self.load_from_file(&vars_file) {
Some(UniversalReadUpdate {
export_generation_increment,
new_vars,
callbacks,
ok_to_save,
}) => {
self.export_generation += export_generation_increment;
self.vars = new_vars;
self.ok_to_save = ok_to_save;
if self.ok_to_save {
(self.save(&directory), Some(callbacks))
} else {
(true, Some(callbacks))
let rewrite = |old_file: &File,
tmp_file: &mut File|
-> std::io::Result<PotentialUpdate<Option<UniversalReadUpdate>>> {
match self.load_from_file(old_file) {
Some(potential_update) => {
if potential_update.do_save {
let contents = Self::serialize_with_vars(&potential_update.data.new_vars);
tmp_file.write_all(&contents)?;
}
Ok(PotentialUpdate {
do_save: potential_update.do_save,
data: Some(potential_update.data),
})
}
None => {
if self.ok_to_save {
let contents = Self::serialize_with_vars(&self.vars);
tmp_file.write_all(&contents)?;
}
Ok(PotentialUpdate {
do_save: self.ok_to_save,
data: None,
})
}
}
None => {
if self.ok_to_save {
(self.save(&directory), None)
} else {
(true, None)
};
let real_path = wrealpath(&self.vars_path).unwrap_or_else(|| self.vars_path.clone());
match rewrite_via_temporary_file(&real_path, &LOCK_UVAR_FILE, rewrite) {
Ok((file_id, potential_update)) => {
self.last_read_file_id = file_id;
self.ok_to_save = potential_update.do_save;
self.modified.clear();
match potential_update.data {
Some(UniversalReadUpdate {
export_generation_increment,
new_vars,
callbacks,
ok_to_save,
}) => {
assert_eq!(potential_update.do_save, ok_to_save);
self.export_generation += export_generation_increment;
self.vars = new_vars;
(true, Some(callbacks))
}
None => (true, None),
}
}
Err(e) => {
FLOG!(uvar_file, "universal log sync failed:", e);
(false, None)
}
}
}
@@ -400,32 +378,46 @@ fn load_from_path_narrow(&mut self) -> Option<CallbackDataList> {
return None;
}
let Ok(file) = open_cloexec(&self.narrow_vars_path, OFlag::O_RDONLY, Mode::empty()) else {
return None;
};
FLOG!(uvar_file, "universal log reading from file");
// TODO: proper locking
match self.load_from_file(&file) {
Some(UniversalReadUpdate {
export_generation_increment,
new_vars,
callbacks,
ok_to_save,
}) => {
match lock_and_load(&self.vars_path, &LOCK_UVAR_FILE, |f| {
Ok(self.load_from_file(f).map(|update| update.data))
}) {
Ok((
file_id,
Some(UniversalReadUpdate {
export_generation_increment,
new_vars,
callbacks,
ok_to_save,
}),
)) => {
self.export_generation += export_generation_increment;
self.vars = new_vars;
self.ok_to_save = ok_to_save;
self.last_read_file_id = file_id;
Some(callbacks)
}
None => None,
Ok((_, None)) => {
// WARNING: Do not update self.file_id here, as this would interfere with
// self.ok_to_save.
None
}
Err(e) => {
FLOG!(uvar_file, "Failed to load from universal variable file:", e);
None
}
}
}
/// Load environment variables from the opened [`file`](`File`).
/// If successful, [`None`] indicates that no updates were necessary, whereas the [`Some`]
/// variant contains updated data.
fn load_from_file(&mut self, file: &File) -> Option<UniversalReadUpdate> {
/// Load environment variables from the [`file`](`File`).
/// If the file has not been modified since the last read, [`None`] is returned.
/// In this case, `self.ok_to_save` needs to be consulted to decide whether to rewrite the file.
/// Otherwise, the [`Some`] variant contains updated data and indicates whether the file should
/// be rewritten..
// IMPORTANT: Callers of this code assume that a return value of None means that the file id has
// not changed. Do not return None in other situations without modifying the callers
// accordingly. Otherwise, problems with self.ok_to_save are expected to occur.
fn load_from_file(&self, file: &File) -> Option<PotentialUpdate<UniversalReadUpdate>> {
// Get the dev / inode.
let current_file_id = file_id_for_file(file);
if current_file_id == self.last_read_file_id {
@@ -438,7 +430,7 @@ fn load_from_file(&mut self, file: &File) -> Option<UniversalReadUpdate> {
// Hacky: if the read format is in the future, avoid overwriting the file: never try to
// save.
let ok_to_save = format != UvarFormat::future;
let do_save = format != UvarFormat::future;
// Announce changes and update our exports generation.
let (export_generation_increment, callbacks) =
@@ -446,126 +438,18 @@ fn load_from_file(&mut self, file: &File) -> Option<UniversalReadUpdate> {
// Acquire the new variables.
self.acquire_variables(&mut new_vars);
self.last_read_file_id = current_file_id;
Some(UniversalReadUpdate {
export_generation_increment,
new_vars,
callbacks,
ok_to_save,
Some(PotentialUpdate {
do_save,
data: UniversalReadUpdate {
export_generation_increment,
new_vars,
callbacks,
ok_to_save: do_save,
},
})
}
}
// Functions concerned with saving.
fn open_and_acquire_lock(&mut self) -> Option<File> {
// Attempt to open the file for reading at the given path, atomically acquiring a lock. On BSD,
// we can use O_EXLOCK. On Linux, we open the file, take a lock, and then compare fstat() to
// stat(); if they match, it means that the file was not replaced before we acquired the lock.
//
// We pass O_RDONLY with O_CREAT; this creates a potentially empty file. We do this so that we
// have something to lock on.
let mut locked_by_open = false;
let mut flags = OFlag::O_RDWR | OFlag::O_CREAT;
if !O_EXLOCK.is_empty() && self.do_flock {
flags |= O_EXLOCK;
locked_by_open = true;
}
loop {
let mut file =
match wopen_cloexec(&self.vars_path, flags, Mode::from_bits_truncate(0o644)) {
Ok(file) => file,
Err(nix::Error::EINTR) => continue,
Err(err) => {
if !O_EXLOCK.is_empty() {
if flags.contains(O_EXLOCK)
&& [nix::Error::ENOTSUP, nix::Error::EOPNOTSUPP].contains(&err)
{
// Filesystem probably does not support locking. Give up on locking.
// Note that on Linux the two errno symbols have the same value but on BSD they're
// different.
flags &= !O_EXLOCK;
self.do_flock = false;
locked_by_open = false;
continue;
}
}
FLOG!(
error,
wgettext_fmt!(
"Unable to open universal variable file '%s': %s",
&self.vars_path,
err.to_string()
)
);
return None;
}
};
// Lock if we want to lock and open() didn't do it for us.
// If flock fails, give up on locking forever.
if self.do_flock && !locked_by_open {
if !flock_uvar_file(&mut file) {
self.do_flock = false;
}
}
// Hopefully we got the lock. However, it's possible the file changed out from under us
// while we were waiting for the lock. Make sure that didn't happen.
if file_id_for_file(&file) != file_id_for_path(&self.vars_path) {
// Oops, it changed! Try again.
drop(file);
} else {
return Some(file);
}
}
}
/// Writes our state to the [`file`](`File`). path is provided only for error reporting.
fn write_to_file(&mut self, file: &mut File, path: &wstr) -> std::io::Result<()> {
let contents = Self::serialize_with_vars(&self.vars);
let res = file.write_all(&contents);
match res.as_ref() {
Ok(()) => {
// Since we just wrote out this file, it matches our internal state; pretend we read from it.
self.last_read_file_id = file_id_for_file(file);
}
Err(err) => {
let error = Errno(err.raw_os_error().unwrap());
FLOG!(
error,
wgettext_fmt!(
"Unable to write to universal variables file '%ls': %s",
path,
error.to_string()
),
);
}
}
// We don't close the file.
res
}
fn move_new_vars_file_into_place(&mut self, src: &wstr, dst: &wstr) -> bool {
let ret = wrename(src, dst);
if ret != 0 {
let error = errno();
FLOG!(
error,
wgettext_fmt!(
"Unable to rename file from '%ls' to '%ls': %s",
src,
dst,
error.to_string()
)
);
}
ret == 0
}
/// Given a variable table, generate callbacks representing the difference between our vars and
/// the new vars.
/// Returns by how much the exports generation count should be incremented, as well as a
@@ -746,85 +630,6 @@ fn read_message_internal(file: &File, vars: &mut VarTable) -> UvarFormat {
Self::populate_variables(&contents, vars)
}
// Write our file contents.
// Return true on success, false on failure.
fn save(&mut self, directory: &wstr) -> bool {
use crate::common::ScopeGuard;
assert!(self.ok_to_save, "It's not OK to save");
// Open adjacent temporary file.
let tmp_name_template = directory.to_owned() + L!("/fishd.tmp.XXXXXX");
let (mut private_file, private_file_path) = match create_temporary_file(&tmp_name_template)
{
Ok(tmp_file_data) => tmp_file_data,
Err(e) => {
FLOG!(warning, "Could not create temporary file:", e);
return false;
}
};
// unlink pfp upon failure. In case of success, it (already) won't exist.
let delete_pfp = ScopeGuard::new(private_file_path, |path| {
wunlink(&path);
});
let private_file_path = &delete_pfp;
// Write to it.
if let Err(e) = self.write_to_file(&mut private_file, private_file_path) {
FLOG!(uvar_file, "universal log write_to_file() failed:", e);
return false;
}
let real_path = wrealpath(&self.vars_path).unwrap_or_else(|| self.vars_path.clone());
// Ensure we maintain ownership and permissions (#2176).
if let Ok(md) = wstat(&real_path) {
// TODO(MSRV): Consider replacing with std::os::unix::fs::fchown when MSRV >= 1.73
if unsafe { libc::fchown(private_file.as_raw_fd(), md.uid(), md.gid()) } == -1 {
FLOG!(uvar_file, "universal log fchown() failed:", errno());
}
if let Err(e) = private_file.set_permissions(md.permissions()) {
FLOG!(uvar_file, "universal log setting permissions failed:", e);
}
}
// Linux by default stores the mtime with low precision, low enough that updates that occur
// in quick succession may result in the same mtime (even the nanoseconds field). So
// manually set the mtime of the new file to a high-precision clock. Note that this is only
// necessary because Linux aggressively reuses inodes, causing the ABA problem; on other
// platforms we tend to notice the file has changed due to a different inode (or file size!)
//
// The current time within the Linux kernel is cached, and generally only updated on a timer
// interrupt. So if the timer interrupt is running at 10 milliseconds, the cached time will
// only be updated once every 10 milliseconds.
//
// It's probably worth finding a simpler solution to this. The tests ran into this, but it's
// unlikely to affect users.
#[cfg(any(target_os = "linux", target_os = "android"))]
{
let mut times: [libc::timespec; 2] = unsafe { std::mem::zeroed() };
times[0].tv_nsec = libc::UTIME_OMIT; // don't change ctime
if unsafe { libc::clock_gettime(libc::CLOCK_REALTIME, &mut times[1]) } == 0 {
unsafe {
libc::futimens(private_file.as_raw_fd(), &times[0]);
}
}
}
// Apply new file.
if !self.move_new_vars_file_into_place(private_file_path, &real_path) {
FLOG!(
uvar_file,
"universal log move_new_vars_file_into_place() failed"
);
return false;
}
// Success at last. All of our modified variables have now been written out.
self.modified.clear();
ScopeGuard::cancel(delete_pfp);
true
}
}
/// Return the default variable path, or an empty string on failure.
@@ -1005,29 +810,6 @@ fn encode_serialized(vals: &[WString]) -> WString {
join_strings(vals, UVAR_ARRAY_SEP)
}
/// Try locking the file.
/// Return true on success, false on error.
fn flock_uvar_file(file: &mut File) -> bool {
let start_time = timef();
while unsafe { libc::flock(file.as_raw_fd(), LOCK_EX) } == -1 {
if errno().0 != EINTR {
return false; // do nothing per issue #2149
}
}
let duration = timef() - start_time;
if duration > 0.25 {
FLOG!(
warning,
wgettext_fmt!(
"Locking the universal var file took too long (%.3f seconds).",
duration
)
);
return false;
}
true
}
fn skip_spaces(mut s: &wstr) -> &wstr {
while s.starts_with(L!(" ")) || s.starts_with(L!("\t")) {
s = &s[1..];

2
src/flog.rs
View File

@@ -124,6 +124,8 @@ pub fn all_categories() -> Vec<&'static category_t> {
(profile_history, "profile-history", "History performance measurements");
(synced_file_access, "synced-file-access", "Synchronized file access");
(iothread, "iothread", "Background IO thread events");
(fd_monitor, "fd-monitor", "FD monitor events");

397
src/fs.rs
View File

@@ -1,10 +1,26 @@
use crate::{
common::{str2wcstring, wcs2zstring},
common::{str2wcstring, wcs2zstring, ScopeGuard},
fds::wopen_cloexec,
global_safety::RelaxedAtomicBool,
path::{path_remoteness, DirRemoteness},
wchar::prelude::*,
FLOG,
wutil::{
file_id_for_file, file_id_for_path, wdirname, wrename, wunlink, FileId, INVALID_FILE_ID,
},
FLOG, FLOGF,
};
use errno::errno;
use std::{ffi::CString, fs::File, os::fd::FromRawFd};
use libc::{c_int, fchown, flock, LOCK_EX, LOCK_SH};
use nix::{fcntl::OFlag, sys::stat::Mode};
use std::{
ffi::CString,
fs::File,
io::Seek,
os::{
fd::{AsRawFd, FromRawFd},
unix::fs::MetadataExt,
},
};
// Replacement for mkostemp(str, O_CLOEXEC)
// This uses mkostemp if available,
@@ -56,3 +72,378 @@ pub fn create_temporary_file(name_template: &wstr) -> std::io::Result<(File, WSt
};
Ok((fd, str2wcstring(c_string_template.to_bytes())))
}
/// Use this struct for all accesses to file which need mutual exclusion.
/// Otherwise, races on the file are possible.
/// The lock is released when this struct is dropped.
pub struct LockedFile {
/// This is the file which requires mutual exclusion.
/// It should only be accessed through this struct,
/// because the locks used here do not protect from other accesses to the file.
data_file: File,
/// The file descriptor of the parent directory, used for locking.
/// The lock is not placed on the data file directly due to issues with renaming.
/// If the data file is renamed after opening and before locking it,
/// There are two independent files around, whose locks do not interact.
/// In some cases this can be identified by checking file identifiers and timestamps,
/// but even with such checks races and corresponding file corruption can occur.
/// It is simpler to lock a different path, which does not change.
///
/// We may fail to lock (e.g. on lockless NFS - see issue #685.
/// In that case, we proceed as if locking succeeded.
/// This might result in corruption,
/// but the alternative of not being able to access the file at all is not desirable either.
_locked_fd: File,
}
pub const LOCKED_FILE_MODE: Mode = Mode::from_bits_truncate(0o600);
pub enum LockingMode {
Shared,
Exclusive(WriteMethod),
}
pub enum WriteMethod {
Append,
RenameIntoPlace,
}
impl LockingMode {
pub fn flock_op(&self) -> c_int {
match self {
Self::Shared => LOCK_SH,
Self::Exclusive(_) => LOCK_EX,
}
}
pub fn file_flags(&self) -> OFlag {
match self {
Self::Shared => OFlag::O_RDONLY,
Self::Exclusive(WriteMethod::Append) => {
OFlag::O_WRONLY | OFlag::O_APPEND | OFlag::O_CREAT
}
Self::Exclusive(WriteMethod::RenameIntoPlace) => OFlag::O_RDONLY | OFlag::O_CREAT,
}
}
}
impl LockedFile {
/// Creates a [`LockedFile`].
/// Use this for any access to a which requires mutual exclusion, as it ensures correct locking.
/// Use [`LockingMode::Exclusive`] if you want to modify the file in any way.
/// Otherwise you should use [`LockingMode::Shared`].
/// Two modes of modification are supported:
/// - Appending
/// - Writing to a temporary file which is then renamed into place.
/// File flags are derived from the [`LockingMode`].
/// `file_name` should just be a name, not a full path.
pub fn new(locking_mode: LockingMode, file_path: &wstr) -> std::io::Result<Self> {
let dir_path = wdirname(file_path);
if path_remoteness(dir_path) == DirRemoteness::remote {
return Err(std::io::Error::new(
std::io::ErrorKind::Unsupported,
"Directory considered remote. Locking is disabled on remote file systems.",
));
}
// We start by locking the directory.
// This is required to avoid racing modifications by other threads/processes.
let dir_fd = wopen_cloexec(dir_path, OFlag::O_RDONLY, Mode::empty())?;
// Try locking the directory. Retry if locking was interrupted.
while unsafe { flock(dir_fd.as_raw_fd(), locking_mode.flock_op()) } == -1 {
let err = std::io::Error::last_os_error();
if err.kind() != std::io::ErrorKind::Interrupted {
return Err(err);
}
}
// Open the data file
let data_file = wopen_cloexec(file_path, locking_mode.file_flags(), LOCKED_FILE_MODE)?;
Ok(Self {
data_file,
_locked_fd: dir_fd,
})
}
pub fn get(&self) -> &File {
&self.data_file
}
pub fn get_mut(&mut self) -> &mut File {
&mut self.data_file
}
}
/// Runs the `load` function, which should see a consistent state of the file at `path`.
/// To ensure a consistent state, we use locks to prevent modifications by others.
/// If locking is unavailable, the `load` function might be executed multiple times,
/// until it manages a run without the file being modified in the meantime, or until the maximum
/// number of allowed attempts is reached.
/// If the file does not exist this function will return an error.
pub fn lock_and_load<F, UserData>(
path: &wstr,
locking_enabled: &RelaxedAtomicBool,
load: F,
) -> std::io::Result<(FileId, UserData)>
where
F: Fn(&File) -> std::io::Result<UserData>,
{
if locking_enabled.load() {
match LockedFile::new(LockingMode::Shared, path) {
Ok(locked_file) => {
return Ok((
file_id_for_file(locked_file.get()),
load(locked_file.get())?,
));
}
Err(e) => {
FLOGF!(
synced_file_access,
"Error acquiring shared lock on the directory of '%s': %s",
path,
e,
);
// This function might be called when the file does not exist.
// Do not abandon locking is such cases.
if e.kind() == std::io::ErrorKind::NotFound {
// There is no point in continuing in this function if the file does not
// exist.
return Err(e);
} else {
FLOG!(synced_file_access, "Disabling locking.");
locking_enabled.store(false);
}
}
}
}
FLOG!(
synced_file_access,
"flock-based locking is disabled. Using fallback implementation."
);
// Fallback implementation for situations where locking is unavailable.
let max_attempts = 1000;
for _ in 0..max_attempts {
let initial_file_id = file_id_for_path(path);
// If we cannot open the file, there is nothing we can do,
// so just return immediately.
let file = wopen_cloexec(path, OFlag::O_RDONLY, Mode::empty())?;
let loaded_data = match load(&file) {
Ok(update_data) => update_data,
Err(_) => {
// Retry if load function failed. Because we do not hold a lock, this might be
// caused by concurrent modifications.
continue;
}
};
let final_file_id = file_id_for_path(path);
if initial_file_id != final_file_id {
continue;
}
// If the file id did not change, we assume that we loaded a consistent state.
return Ok((final_file_id, loaded_data));
}
Err(std::io::Error::new(std::io::ErrorKind::Other, "Failed to update the file. Locking is disabled, and the fallback code did not succeed within the permissible number of attempts."))
}
pub struct PotentialUpdate<UserData> {
pub do_save: bool,
pub data: UserData,
}
/// Use this function for updating a file based on its current content,
/// for files which might be accessed at the same time by other threads/processes.
/// The basic principle is to create a temporary file, take a lock on the file to be rewritten,
/// do the rewrite (which might involve reading the old file contents),
/// and finally release the lock.
/// Error handling (especially the case where locking is not possible) makes the implementation
/// non-straightforward.
///
/// # Arguments
///
/// - `path`: The path to the file which should be updated.
/// - `locking_enabled`: To indicate whether locking should be attempted. This function might
/// update the value stored in `locking_enabled`.
/// - `rewrite`: The function which handles reading from the file and writing to a temporary file.
/// The first argument is for the file to read from, the second for the temporary file to write to.
/// On success, the value returned by `rewrite` is included in this functions return value. Be
/// careful about side effects of `rewrite`. It might get executed multiple times. Try to avoid
/// side effects and instead extract any data you might need and return them on success.
/// Then, apply the desired side effects once this function has returned successfully.
///
/// # Return value
///
/// On success, the [`FileId`] of the rewritten file is returned, alongside the value returned by
/// `rewrite`. Note that if locking is unavailable, the [`FileId`] might be the id of a different
/// version of the file, which was written after this function renamed the temporary file to `path`
/// but before we obtained the [`FileId`] from `path`. This is a race condition we do not detect.
pub fn rewrite_via_temporary_file<F, UserData>(
path: &wstr,
locking_enabled: &RelaxedAtomicBool,
rewrite: F,
) -> std::io::Result<(FileId, PotentialUpdate<UserData>)>
where
F: Fn(&File, &mut File) -> std::io::Result<PotentialUpdate<UserData>>,
{
/// Updates the metadata of the `new_file` to match the `old_file`.
/// Also updates the mtime of the `new_file` to the current time manually, to work around
/// operating systems which do not update the internal time used for such time stamps
/// frequently enough. This is important for [`FileId`] comparisons.
fn update_metadata(old_file: &File, new_file: &File) {
// Ensure we maintain the ownership and permissions of the original (#2355). If the
// stat fails, we assume (hope) our default permissions are correct. This
// corresponds to e.g. someone running sudo -E as the very first command. If they
// did, it would be tricky to set the permissions correctly. (bash doesn't get this
// case right either).
if let Ok(md) = old_file.metadata() {
// TODO(MSRV): Consider replacing with std::os::unix::fs::fchown when MSRV >= 1.73
if unsafe { fchown(new_file.as_raw_fd(), md.uid(), md.gid()) } == -1 {
FLOG!(
synced_file_access,
"Error when changing ownership of file:",
errno::errno()
);
}
if let Err(e) = new_file.set_permissions(md.permissions()) {
FLOG!(synced_file_access, "Error when changing mode of file:", e);
}
} else {
FLOG!(synced_file_access, "Could not get metadata for file");
}
// Linux by default stores the mtime with low precision, low enough that updates that occur
// in quick succession may result in the same mtime (even the nanoseconds field). So
// manually set the mtime of the new file to a high-precision clock. Note that this is only
// necessary because Linux aggressively reuses inodes, causing the ABA problem; on other
// platforms we tend to notice the file has changed due to a different inode.
//
// The current time within the Linux kernel is cached, and generally only updated on a timer
// interrupt. So if the timer interrupt is running at 10 milliseconds, the cached time will
// only be updated once every 10 milliseconds.
#[cfg(any(target_os = "linux", target_os = "android"))]
{
let mut times: [libc::timespec; 2] = unsafe { std::mem::zeroed() };
times[0].tv_nsec = libc::UTIME_OMIT; // don't change atime
if unsafe { libc::clock_gettime(libc::CLOCK_REALTIME, &mut times[1]) } == 0 {
unsafe {
// This accesses both times[0] and times[1]. Check `utimensat(2)` for details.
libc::futimens(new_file.as_raw_fd(), &times[0]);
}
}
}
}
/// Renames a file from `old_name` to `new_name`.
fn rename(old_name: &wstr, new_name: &wstr) -> std::io::Result<()> {
if wrename(old_name, new_name) == -1 {
let error_number = errno::errno();
FLOG!(
error,
wgettext_fmt!("Error when renaming file: %s", error_number.to_string())
);
return Err(std::io::Error::from(error_number));
}
Ok(())
}
const TMP_FILE_SUFFIX: &wstr = L!(".XXXXXX");
let tmp_file_template = path.to_owned() + TMP_FILE_SUFFIX;
let (tmp_file, tmp_name) = create_temporary_file(&tmp_file_template)?;
// Ensure that the temporary file is unlinked when this function returns.
let mut tmp_file = ScopeGuard::new(tmp_file, |_| {
wunlink(&tmp_name);
});
// We want to rewrite the file.
// To avoid issues with crashes during writing,
// we write to a temporary file and once we are done, this file is renamed such that it
// replaces the original file.
if locking_enabled.load() {
// To avoid races, we need to have exclusive access to the file for the entire
// duration, which we get via an exclusive lock on the parent directory.
// Taking a shared lock first and later upgrading to an exclusive one could result in a
// deadlock, so we take an exclusive one immediately.
match LockedFile::new(LockingMode::Exclusive(WriteMethod::RenameIntoPlace), path) {
Ok(locked_file) => {
let potential_update = rewrite(locked_file.get(), &mut tmp_file)?;
if potential_update.do_save {
update_metadata(locked_file.get(), &tmp_file);
rename(&tmp_name, path)?;
}
return Ok((file_id_for_path(path), potential_update));
}
Err(e) => {
FLOGF!(
synced_file_access,
"Error acquiring exclusive lock on the directory of '%s': %s",
path,
e,
);
locking_enabled.store(false);
}
}
}
// If this is reached, we assume that locking is not available so we use a fallback
// implementation which tries to avoid race conditions, but in the case of contention it is
// possible that some writes are lost.
FLOG!(
synced_file_access,
"flock-based locking is disabled. Using fallback implementation."
);
// Give up after this many unsuccessful attempts.
// TODO: which value, should this be a constant shared with other retrying logic?
let max_attempts = 1000;
for _ in 0..max_attempts {
// Truncate tmp_file for the next attempt to get rid of data potentially written in the
// previous iteration.
tmp_file.set_len(0)?;
tmp_file.seek(std::io::SeekFrom::Start(0)).unwrap();
// If the file does not exist yet, this will be `INVALID_FILE_ID`.
let initial_file_id = file_id_for_path(path);
// If we cannot open the file, there is nothing we can do,
// so just return immediately.
let old_file = wopen_cloexec(path, OFlag::O_RDONLY | OFlag::O_CREAT, LOCKED_FILE_MODE)?;
let opened_file_id = file_id_for_file(&old_file);
if initial_file_id != INVALID_FILE_ID && initial_file_id != opened_file_id {
// File ID changed (and not just because the file was created by us).
continue;
}
let Ok(potential_update) = rewrite(&old_file, &mut tmp_file) else {
// Retry if rewrite function failed. Because we do not hold a lock, this might be
// caused by concurrent modifications.
continue;
};
if potential_update.do_save {
update_metadata(&old_file, &tmp_file);
}
let mut final_file_id = file_id_for_path(path);
if opened_file_id != final_file_id {
continue;
}
// If we reach this point, the file ID did not change while we read the old file and wrote
// to `tmp_file`. Now we replace the old file with the `tmp_file`.
// Note that we cannot prevent races here.
// If the file is modified by someone else between the syscall for determining the [`FileId`]
// and the rename syscall, these modifications will be lost.
if potential_update.do_save {
// Do not retry on rename failures, as it is unlikely that these will disappear if we retry.
rename(&tmp_name, path)?;
final_file_id = file_id_for_path(path);
}
// Note that this might not match the version of the file we just wrote.
// (If we did write.)
return Ok((final_file_id, potential_update));
}
Err(std::io::Error::new(std::io::ErrorKind::Other, "Failed to update the file. Locking is disabled, and the fallback code did not succeed within the permissible number of attempts."))
}

489
src/history.rs
View File

@@ -2,47 +2,41 @@
//!
//! 1. All history files are append-only. Data, once written, is never modified.
//!
//! 2. A history file may be re-written ("vacuumed"). This involves reading in the file and writing a
//! new one, while performing maintenance tasks: discarding items in an LRU fashion until we reach
//! the desired maximum count, removing duplicates, and sorting them by timestamp (eventually, not
//! implemented yet). The new file is atomically moved into place via rename().
//! 2. A history file may be re-written ("vacuumed"). This involves reading in the file and writing
//! a new one, while performing maintenance tasks: discarding items in an LRU fashion until we
//! reach the desired maximum count, removing duplicates, and sorting them by timestamp
//! (eventually, not implemented yet). The new file is atomically moved into place via `rename()`.
//!
//! 3. History files are mapped in via mmap(). Before the file is mapped, the file takes a fcntl read
//! lock. The purpose of this lock is to avoid seeing a transient state where partial data has been
//! written to the file.
//! 3. History files are mapped in via `mmap()`. This allows only storing one `usize` per item (its
//! offset), and lazily loading items on demand, which reduces memory consumption.
//!
//! 4. History is appended to under a fcntl write lock.
//!
//! 5. The chaos_mode boolean can be set to true to do things like lower buffer sizes which can
//! trigger race conditions. This is useful for testing.
//!
//! Locking on remote filesystems may hang for an unacceptably long time. For that reason, fish
//! does not take locks on the file if it believes the history file is on a remote filesystem,
//! or if the mmap fails with ENODEV, or if the first lock attempt takes excessively long.
//! Eliding locks means that two concurrent shell sessions with a remote history file may, in
//! rare cases with multiple simultaneous shell sessions, lose a history item; this is
//! considered preferable to hanging the the shell waiting for a lock.
//! 4. Accesses to the history file need to be synchronized. This is achieved by functionality in
//! `src/fs.rs`. By default, `flock()` is used for locking. If that is unavailable, an imperfect
//! fallback solution attempts to detect races and retries if a race is detected.
use crate::{
common::cstr2wcstring, env::EnvVar, fs::create_temporary_file, wcstringutil::trim,
wutil::fileid::file_id_for_path_or_error,
common::cstr2wcstring,
env::EnvVar,
fs::{
lock_and_load, rewrite_via_temporary_file, LockedFile, LockingMode, PotentialUpdate,
WriteMethod, LOCKED_FILE_MODE,
},
wcstringutil::trim,
};
use std::{
borrow::Cow,
collections::{BTreeMap, HashMap, HashSet},
ffi::CString,
fs::File,
io::{BufRead, Read, Seek, SeekFrom, Write},
io::{BufRead, Read, Write},
mem::MaybeUninit,
num::NonZeroUsize,
ops::ControlFlow,
os::{fd::AsRawFd, unix::fs::MetadataExt},
sync::{Arc, Mutex, MutexGuard},
time::{Duration, SystemTime, UNIX_EPOCH},
};
use bitflags::bitflags;
use libc::{fchown, flock, EINTR, LOCK_EX, LOCK_SH, LOCK_UN};
use lru::LruCache;
use nix::{fcntl::OFlag, sys::stat::Mode};
use rand::Rng;
@@ -60,18 +54,13 @@
operation_context::{OperationContext, EXPANSION_LIMIT_BACKGROUND},
parse_constants::{ParseTreeFlags, StatementDecoration},
parse_util::{parse_util_detect_errors, parse_util_unescape_wildcards},
path::{
path_get_config, path_get_data, path_get_data_remoteness, path_is_valid, DirRemoteness,
},
path::{path_get_config, path_get_data, path_is_valid},
threads::{assert_is_background_thread, iothread_perform},
util::{find_subslice, get_rng},
wchar::prelude::*,
wcstringutil::subsequence_in_string,
wildcard::{wildcard_match, ANY_STRING},
wutil::{
file_id_for_file, file_id_for_path, wgettext_fmt, wrealpath, wrename, wstat, wunlink,
FileId, INVALID_FILE_ID,
},
wutil::{file_id_for_file, wgettext_fmt, wrealpath, wstat, wunlink, FileId, INVALID_FILE_ID},
};
mod file;
@@ -135,14 +124,6 @@ pub enum SearchDirection {
/// Default buffer size for flushing to the history file.
const HISTORY_OUTPUT_BUFFER_SIZE: usize = 64 * 1024;
/// The file access mode we use for creating history files
const HISTORY_FILE_MODE: Mode = Mode::from_bits_truncate(0o600);
/// How many times we retry to save
/// Saving may fail if the file is modified in between our opening
/// the file and taking the lock
const MAX_SAVE_TRIES: usize = 1024;
pub const VACUUM_FREQUENCY: usize = 25;
/// Output the contents `buffer` to `file` and clear the `buffer`.
@@ -215,29 +196,6 @@ fn add_item(&mut self, item: HistoryItem) {
}
}
/// Returns the path for the history file for the given `session_id`
/// if `session_id` is non-empty.
/// If it is empty, `Ok(None)` will be returned.
/// An error is returned if obtaining the data directory failed.
/// Because the `path_get_data` function does not return error information,
/// we cannot provide more detail about the reason for the failure here.
/// If `suffix` is provided, append that suffix to the path; this is used for temporary files.
fn history_filename(session_id: &wstr, suffix: &wstr) -> std::io::Result<Option<WString>> {
if session_id.is_empty() {
return Ok(None);
}
let Some(mut result) = path_get_data() else {
return Err(std::io::Error::new(std::io::ErrorKind::NotFound, "Error obtaining data directory. This is a manually constructed error which does not indicate why this happened."));
};
result.push('/');
result.push_utfstr(session_id);
result.push_utfstr(L!("_history"));
result.push_utfstr(suffix);
Ok(Some(result))
}
pub type PathList = Vec<WString>;
pub type HistoryIdentifier = u64;
@@ -406,11 +364,33 @@ struct HistoryImpl {
old_item_offsets: Vec<usize>,
}
/// If set, we gave up on file locking because it took too long.
/// Note this is shared among all history instances.
static ABANDONED_LOCKING: RelaxedAtomicBool = RelaxedAtomicBool::new(false);
impl HistoryImpl {
/// Returns the canonical path for the history file, or `Ok(None)` in private mode.
/// An error is returned if obtaining the data directory fails.
/// Because the `path_get_data` function does not return error information,
/// we cannot provide more detail about the reason for the failure here.
fn history_file_path(&self) -> std::io::Result<Option<WString>> {
if self.name.is_empty() {
return Ok(None);
}
let Some(mut path) = path_get_data() else {
return Err(std::io::Error::new(std::io::ErrorKind::NotFound, "Error obtaining data directory. This is a manually constructed error which does not indicate why this happened."));
};
path.push('/');
path.push_utfstr(&self.name);
path.push_utfstr(L!("_history"));
if let Some(canonicalized_path) = wrealpath(&path) {
Ok(Some(canonicalized_path))
} else {
Err(std::io::Error::new(
std::io::ErrorKind::Other,
format!("wrealpath failed to produce a canonical version of '{path}'."),
))
}
}
/// Add a new history item to the end. If `pending` is set, the item will not be returned by
/// `item_at_index()` until a call to `resolve_pending()`. Pending items are tracked with an
/// offset into the array of new items, so adding a non-pending item has the effect of resolving
@@ -502,34 +482,22 @@ fn load_old_if_needed(&mut self) {
self.loaded_old = true;
let _profiler = TimeProfiler::new("load_old");
if let Ok(Some(history_path)) = history_filename(&self.name, L!("")) {
let Ok(mut file) = wopen_cloexec(&history_path, OFlag::O_RDONLY, Mode::empty()) else {
return;
};
// Take a read lock to guard against someone else appending. This is released after
// getting the file's length. We will read the file after releasing the lock, but that's
// not a problem, because we never modify already written data. In short, the purpose of
// this lock is to ensure we don't see the file size change mid-update.
//
// We may fail to lock (e.g. on lockless NFS - see issue #685. In that case, we proceed
// as if it did not fail. The risk is that we may get an incomplete history item; this
// is unlikely because we only treat an item as valid if it has a terminating newline.
let locked = unsafe { Self::maybe_lock_file(&mut file, LOCK_SH) };
self.file_contents = HistoryFileContents::create(&file).ok();
self.history_file_id = if self.file_contents.is_some() {
file_id_for_file(&file)
} else {
INVALID_FILE_ID
};
if locked {
unsafe {
Self::unlock_file(&mut file);
if let Ok(Some(history_path)) = self.history_file_path() {
match lock_and_load(
&history_path,
&LOCK_HISTORY_FILE,
HistoryFileContents::create,
) {
Ok((file_id, file_contents)) => {
self.file_contents = Some(file_contents);
self.history_file_id = file_id;
let _profiler = TimeProfiler::new("populate_from_file_contents");
self.populate_from_file_contents();
}
Err(e) => {
FLOG!(history_file, "Error reading from history file:", e);
}
}
let _profiler = TimeProfiler::new("populate_from_file_contents");
self.populate_from_file_contents();
}
}
@@ -578,10 +546,9 @@ fn remove_ephemeral_items(&mut self) {
}
/// Given an existing history file, write a new history file to `dst`.
/// Returns false on error, true on success
fn rewrite_to_temporary_file(
&self,
existing_file: Option<&mut File>,
existing_file: &File,
dst: &mut File,
) -> std::io::Result<()> {
// We are reading FROM existing_file and writing TO dst
@@ -591,32 +558,29 @@ fn rewrite_to_temporary_file(
// Read in existing items (which may have changed out from underneath us, so don't trust our
// old file contents).
if let Some(existing_file) = existing_file {
if let Ok(local_file) = HistoryFileContents::create(existing_file) {
let mut cursor = 0;
while let Some(offset) = local_file.offset_of_next_item(&mut cursor, None) {
// Try decoding an old item.
let Some(old_item) = local_file.decode_item(offset) else {
continue;
};
if let Ok(local_file) = HistoryFileContents::create(existing_file) {
let mut cursor = 0;
while let Some(offset) = local_file.offset_of_next_item(&mut cursor, None) {
// Try decoding an old item.
let Some(old_item) = local_file.decode_item(offset) else {
continue;
};
// If old item is newer than session always erase if in deleted.
if old_item.timestamp() > self.boundary_timestamp {
if old_item.is_empty() || self.deleted_items.contains_key(old_item.str()) {
continue;
}
lru.add_item(old_item);
} else {
// If old item is older and in deleted items don't erase if added by
// clear_session.
if old_item.is_empty()
|| self.deleted_items.get(old_item.str()) == Some(&false)
{
continue;
}
// Add this old item.
lru.add_item(old_item);
// If old item is newer than session always erase if in deleted.
if old_item.timestamp() > self.boundary_timestamp {
if old_item.is_empty() || self.deleted_items.contains_key(old_item.str()) {
continue;
}
lru.add_item(old_item);
} else {
// If old item is older and in deleted items don't erase if added by
// clear_session.
if old_item.is_empty() || self.deleted_items.get(old_item.str()) == Some(&false)
{
continue;
}
// Add this old item.
lru.add_item(old_item);
}
}
}
@@ -669,13 +633,9 @@ fn rewrite_to_temporary_file(
/// Saves history by rewriting the file.
fn save_internal_via_rewrite(&mut self) -> std::io::Result<()> {
// We want to rewrite the file, while holding the lock for as briefly as possible
// To do this, we speculatively write a file, and then lock and see if our original file changed
// Repeat until we succeed or give up
let Some(possibly_indirect_target_name) = history_filename(&self.name, L!(""))? else {
let Some(history_path) = self.history_file_path()? else {
return Ok(());
};
let tmp_name_template = history_filename(&self.name, L!(".XXXXXX"))?.unwrap();
FLOGF!(
history,
@@ -683,150 +643,43 @@ fn save_internal_via_rewrite(&mut self) -> std::io::Result<()> {
self.new_items.len() - self.first_unwritten_new_item_index
);
// If the history file is a symlink, we want to rewrite the real file so long as we can find it.
let target_name =
wrealpath(&possibly_indirect_target_name).unwrap_or(possibly_indirect_target_name);
let rewrite =
|old_file: &File, tmp_file: &mut File| -> std::io::Result<PotentialUpdate<()>> {
self.rewrite_to_temporary_file(old_file, tmp_file)?;
Ok(PotentialUpdate {
do_save: true,
data: (),
})
};
// Make our temporary file
let (mut tmp_file, tmp_name) = create_temporary_file(&tmp_name_template)?;
let mut done = false;
for _i in 0..MAX_SAVE_TRIES {
if done {
break;
}
let (file_id, _) = rewrite_via_temporary_file(&history_path, &LOCK_HISTORY_FILE, rewrite)?;
self.history_file_id = file_id;
let target_file_before = wopen_cloexec(
&target_name,
OFlag::O_RDONLY | OFlag::O_CREAT,
HISTORY_FILE_MODE,
);
if let Err(err) = target_file_before {
FLOG!(history_file, "Error opening history file:", err);
}
// We've saved everything, so we have no more unsaved items.
self.first_unwritten_new_item_index = self.new_items.len();
let orig_file_id = target_file_before
.as_ref()
.map(file_id_for_file)
.unwrap_or(INVALID_FILE_ID);
// We deleted our deleted items.
self.deleted_items.clear();
// Open any target file, but do not lock it right away
if let Err(err) =
self.rewrite_to_temporary_file(target_file_before.ok().as_mut(), &mut tmp_file)
{
// Failed to write, no good
FLOG!(history_file, "Error writing to temporary file:", err);
break;
}
// Our history has been written to the file, so clear our state so we can re-reference the
// file.
self.clear_file_state();
// The crux! We rewrote the history file; see if the history file changed while we
// were rewriting it. Make an effort to take the lock before checking, to avoid racing.
// If the open fails, then proceed; this may be because there is no current history
let mut new_file_id = INVALID_FILE_ID;
let mut target_file_after = wopen_cloexec(&target_name, OFlag::O_RDONLY, Mode::empty());
if let Ok(target_file_after) = target_file_after.as_mut() {
// critical to take the lock before checking file IDs,
// and hold it until after we are done replacing.
// Also critical to check the file at the path, NOT based on our fd.
// It's only OK to replace the file while holding the lock.
// Note any lock is released when target_file_after is closed.
unsafe {
Self::maybe_lock_file(target_file_after, LOCK_EX);
}
new_file_id = match file_id_for_path_or_error(&target_name) {
Ok(file_id) => file_id,
Err(err) => {
if err.kind() != std::io::ErrorKind::NotFound {
FLOG!(history_file, "Error re-opening history file:", err);
}
INVALID_FILE_ID
}
}
}
let can_replace_file = new_file_id == orig_file_id || new_file_id == INVALID_FILE_ID;
if !can_replace_file {
// The file has changed, so we're going to re-read it
// Truncate our tmp_file so we can reuse it
if let Err(err) = tmp_file.set_len(0) {
FLOG!(
history_file,
"Error when truncating temporary history file:",
err
);
}
if let Err(err) = tmp_file.seek(SeekFrom::Start(0)) {
FLOG!(
history_file,
"Error resetting cursor in temporary history file:",
err
);
}
} else {
// The file is unchanged, or the new file doesn't exist or we can't read it
// We also attempted to take the lock, so we feel confident in replacing it
// Ensure we maintain the ownership and permissions of the original (#2355). If the
// stat fails, we assume (hope) our default permissions are correct. This
// corresponds to e.g. someone running sudo -E as the very first command. If they
// did, it would be tricky to set the permissions correctly. (bash doesn't get this
// case right either).
if let Ok(target_file_after) = target_file_after.as_ref() {
if let Ok(md) = target_file_after.metadata() {
// TODO(MSRV): Consider replacing with std::os::unix::fs::fchown when MSRV >= 1.73
if unsafe { fchown(tmp_file.as_raw_fd(), md.uid(), md.gid()) } == -1 {
FLOG!(
history_file,
"Error when changing ownership of history file:",
errno::errno()
);
}
if let Err(e) = tmp_file.set_permissions(md.permissions()) {
FLOG!(history_file, "Error when changing mode of history file:", e);
}
}
}
// Slide it into place
if wrename(&tmp_name, &target_name) == -1 {
FLOG!(
error,
wgettext_fmt!(
"Error when renaming history file: %s",
errno::errno().to_string()
)
);
}
// We did it
done = true;
}
}
// Ensure we never leave the old file around
let _ = wunlink(&tmp_name);
if done {
// We've saved everything, so we have no more unsaved items.
self.first_unwritten_new_item_index = self.new_items.len();
// We deleted our deleted items.
self.deleted_items.clear();
// Our history has been written to the file, so clear our state so we can re-reference the
// file.
self.clear_file_state();
}
Ok(())
}
/// Saves history by appending to the file.
fn save_internal_via_appending(&mut self) -> std::io::Result<()> {
// Get the path to the real history file.
let Some(history_path) = history_filename(&self.name, L!(""))? else {
let Some(history_path) = self.history_file_path()? else {
return Ok(());
};
let history_path = wrealpath(&history_path).unwrap_or(history_path);
if !LOCK_HISTORY_FILE.load() {
return Err(std::io::Error::new(
std::io::ErrorKind::Unsupported,
"Appending is not supported when locking is disabled.",
));
}
FLOGF!(
history,
@@ -836,58 +689,20 @@ fn save_internal_via_appending(&mut self) -> std::io::Result<()> {
// No deleting allowed.
assert!(self.deleted_items.is_empty());
// If the file is different (someone vacuumed it) then we need to update our mmap.
let mut file_changed = false;
let mut locked_history_file =
LockedFile::new(LockingMode::Exclusive(WriteMethod::Append), &history_path)?;
// We are going to open the file, lock it, append to it, and then close it
// After locking it, we need to stat the file at the path; if there is a new file there, it
// means the file was replaced and we have to try again.
// Limit our max tries so we don't do this forever.
let mut num_attempts = 0;
let mut history_file = loop {
if num_attempts == MAX_SAVE_TRIES {
return Err(std::io::Error::new(
std::io::ErrorKind::Other,
"Number of unsuccessful attempts to open history file exceeds maximum.",
));
}
// Return immediately if an error occurs here.
let mut file = wopen_cloexec(
&history_path,
OFlag::O_WRONLY | OFlag::O_APPEND,
Mode::empty(),
)?;
// Check if the file was modified since it was last read.
let file_id = file_id_for_file(locked_history_file.get());
let file_changed = file_id != self.history_file_id;
// Exclusive lock on the entire file. This is released when we close the file (below). This
// may fail on (e.g.) lockless NFS. If so, proceed as if it did not fail; the risk is that
// we may get interleaved history items, which is considered better than no history, or
// forcing everything through the slow copy-move mode. We try to minimize this possibility
// by writing with O_APPEND.
unsafe {
Self::maybe_lock_file(&mut file, LOCK_EX);
}
let file_id = file_id_for_file(&file);
if file_id_for_path(&history_path) == file_id {
// File IDs match, so the file we opened is still at that path
// We're going to use this fd
if file_id != self.history_file_id {
file_changed = true;
}
break file;
}
num_attempts += 1;
};
// We (hopefully successfully) took the exclusive lock. Append to the file.
// We took the exclusive lock. Append to the file.
// Note that this is sketchy for a few reasons:
// - Another shell may have appended its own items with a later timestamp, so our file may
// no longer be sorted by timestamp.
// - Another shell may have appended the same items, so our file may now contain
// duplicates.
//
// We cannot modify any previous parts of our file, because other instances may be reading
// those portions. We can only append.
//
// Originally we always rewrote the file on saving, which avoided both of these problems.
// However, appending allows us to save history after every command, which is nice!
//
@@ -911,7 +726,8 @@ fn save_internal_via_appending(&mut self) -> std::io::Result<()> {
// the file system.
// Flushing and syncing each iteration adds overhead,
// but hopefully there are not that many items to write when appending.
res = drain_buffer_into_file_and_flush(&mut buffer, &mut history_file);
res = drain_buffer_into_file_and_flush(&mut buffer, locked_history_file.get_mut());
if res.is_err() {
break;
}
@@ -923,11 +739,11 @@ fn save_internal_via_appending(&mut self) -> std::io::Result<()> {
// Since we just modified the file, update our history_file_id to match its current state
// Otherwise we'll think the file has been changed by someone else the next time we go to
// write.
// We don't update the mapping since we only appended to the file, and everything we
// We don't update `self.file_contents` since we only appended to the file, and everything we
// appended remains in our new_items
self.history_file_id = file_id_for_file(&history_file);
self.history_file_id = file_id;
drop(history_file);
drop(locked_history_file);
// If someone has replaced the file, forget our file state.
if file_changed {
@@ -946,6 +762,9 @@ fn save(&mut self, vacuum: bool) {
return;
}
// Compact our new items so we don't have duplicates.
self.compact_new_items();
if self.name.is_empty() {
// We're in the "incognito" mode. Pretend we've saved the history.
self.first_unwritten_new_item_index = self.new_items.len();
@@ -953,16 +772,13 @@ fn save(&mut self, vacuum: bool) {
self.clear_file_state();
}
// Compact our new items so we don't have duplicates.
self.compact_new_items();
// Try saving. If we have items to delete, we have to rewrite the file. If we do not, we can
// append to it.
let mut ok = false;
if !vacuum && self.deleted_items.is_empty() {
// Try doing a fast append.
if let Err(e) = self.save_internal_via_appending() {
FLOG!(history, "Appending to history failed", e);
FLOG!(history, "Appending to history failed:", e);
} else {
ok = true;
}
@@ -1048,7 +864,7 @@ fn is_empty(&mut self) -> bool {
// If we have not loaded old items, don't actually load them (which may be expensive); just
// stat the file and see if it exists and is nonempty.
let Ok(Some(where_)) = history_filename(&self.name, L!("")) else {
let Ok(Some(where_)) = self.history_file_path() else {
return true;
};
@@ -1104,7 +920,7 @@ fn clear(&mut self) {
self.deleted_items.clear();
self.first_unwritten_new_item_index = 0;
self.old_item_offsets.clear();
if let Ok(Some(filename)) = history_filename(&self.name, L!("")) {
if let Ok(Some(filename)) = self.history_file_path() {
wunlink(&filename);
}
self.clear_file_state();
@@ -1125,7 +941,7 @@ fn clear_session(&mut self) {
/// file to the new history file.
/// The new contents will automatically be re-mapped later.
fn populate_from_config_path(&mut self) {
let Ok(Some(new_file)) = history_filename(&self.name, L!("")) else {
let Ok(Some(new_file)) = self.history_file_path() else {
return;
};
@@ -1148,7 +964,7 @@ fn populate_from_config_path(&mut self) {
let mut dst_file = match wopen_cloexec(
&new_file,
OFlag::O_WRONLY | OFlag::O_CREAT,
HISTORY_FILE_MODE,
LOCKED_FILE_MODE,
) {
Ok(file) => file,
Err(err) => {
@@ -1345,57 +1161,6 @@ fn size(&mut self) -> usize {
let old_item_count = self.old_item_offsets.len();
return new_item_count + old_item_count;
}
/// Maybe lock a history file.
/// Returns `true` if successful, `false` if locking was skipped.
///
/// # Safety
///
/// `fd` and `lock_type` must be valid arguments to `flock(2)`.
unsafe fn maybe_lock_file(file: &mut File, lock_type: libc::c_int) -> bool {
assert!(lock_type & LOCK_UN == 0, "Do not use lock_file to unlock");
// Don't lock if it took too long before, if we are simulating a failing lock, or if our history
// is on a remote filesystem.
if ABANDONED_LOCKING.load() {
return false;
}
if path_get_data_remoteness() == DirRemoteness::remote {
return false;
}
let (ok, start_time) = loop {
let start_time = SystemTime::now();
if unsafe { flock(file.as_raw_fd(), lock_type) } != -1 {
break (true, start_time);
}
if errno::errno().0 != EINTR {
break (false, start_time);
}
};
if let Ok(duration) = start_time.elapsed() {
if duration > Duration::from_millis(250) {
FLOG!(
warning,
wgettext_fmt!(
"Locking the history file took too long (%.3f seconds).",
duration.as_secs_f64()
)
);
ABANDONED_LOCKING.store(true);
}
}
ok
}
/// Unlock a history file.
///
/// # Safety
///
/// `fd` must be a valid argument to `flock(2)` with `LOCK_UN`.
unsafe fn unlock_file(file: &mut File) {
libc::flock(file.as_raw_fd(), LOCK_UN);
}
}
fn string_could_be_path(potential_path: &wstr) -> bool {
@@ -2081,3 +1846,5 @@ pub fn in_private_mode(vars: &dyn Environment) -> bool {
/// Whether to force the read path instead of mmap. This is useful for testing.
static NEVER_MMAP: RelaxedAtomicBool = RelaxedAtomicBool::new(false);
static LOCK_HISTORY_FILE: RelaxedAtomicBool = RelaxedAtomicBool::new(true);

6
src/history/file.rs
View File

@@ -144,7 +144,11 @@ pub fn create(mut history_file: &File) -> std::io::Result<Self> {
// filesystem does not support mapping. Treat this as a hint
// that the filesystem is remote, and so disable locks for
// the history file.
super::ABANDONED_LOCKING.store(true);
FLOG!(
history_file,
"Remote file system detected. Disabling history file locking.",
);
super::LOCK_HISTORY_FILE.store(false);
// Create an anonymous mapping and read() the file into it.
map_anon(history_file, len)?
} else {

2
src/path.rs
View File

@@ -667,7 +667,7 @@ fn create_dir_all_with_mode<P: AsRef<std::path::Path>>(path: P, mode: u32) -> st
}
/// Return whether the given path is on a remote filesystem.
fn path_remoteness(path: &wstr) -> DirRemoteness {
pub fn path_remoteness(path: &wstr) -> DirRemoteness {
let narrow = wcs2zstring(path);
#[cfg(target_os = "linux")]
{

2
src/wutil/tests.rs
View File

@@ -61,7 +61,7 @@ fn test_wdirname_wbasename() {
#[serial]
fn test_wwrite_to_fd() {
let _cleanup = test_init();
let (_fd, filename) = create_temporary_file(L!("/tmp/fish_test_wwrite.XXXXXX")).unwrap();
let (_file, filename) = create_temporary_file(L!("/tmp/fish_test_wwrite.XXXXXX")).unwrap();
let filename = CString::new(filename.to_string()).unwrap();
let mut rng = get_rng();
let sizes = [1, 2, 3, 5, 13, 23, 64, 128, 255, 4096, 4096 * 2];