From 5c8d822ee1146340b003f4d3b36554fdcd5f3c7e Mon Sep 17 00:00:00 2001 From: Daniel Rainer Date: Thu, 6 Mar 2025 19:49:09 +0100 Subject: [PATCH] Flush history with sync - Replace the write_loop call with Writer::write_all. - Call sync_all on the file to ensure it gets written to storage. --- src/history.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/history.rs b/src/history.rs index 3e47d2725..2fbba1912 100644 --- a/src/history.rs +++ b/src/history.rs @@ -36,7 +36,7 @@ num::NonZeroUsize, ops::ControlFlow, os::{ - fd::{AsFd, AsRawFd, RawFd}, + fd::{AsFd, AsRawFd}, unix::fs::MetadataExt, }, sync::{Arc, Mutex, MutexGuard}, @@ -52,7 +52,7 @@ use crate::{ ast::{Ast, Node}, common::{ - str2wcstring, unescape_string, valid_var_name, wcs2zstring, write_loop, CancelChecker, + str2wcstring, unescape_string, valid_var_name, wcs2zstring, CancelChecker, UnescapeStringStyle, }, env::{EnvMode, EnvStack, Environment}, @@ -154,14 +154,18 @@ pub enum SearchDirection { /// If the size of `buffer` is at least `min_size`, output the contents `buffer` to `fd`, /// and clear the string. -fn flush_to_fd(buffer: &mut Vec, fd: RawFd, min_size: usize) -> std::io::Result<()> { +fn flush_to_file(buffer: &mut Vec, file: &mut File, min_size: usize) -> std::io::Result<()> { if buffer.is_empty() || buffer.len() < min_size { return Ok(()); } - write_loop(&fd, buffer)?; + file.write_all(buffer)?; buffer.clear(); - return Ok(()); + // Ensure that the file content is actually written to persistent storage. + // This might be unnecessary. + // It is used to help eliminate possible causes for + // https://github.com/fish-shell/fish-shell/issues/10300 + file.sync_all() } struct TimeProfiler { @@ -636,7 +640,7 @@ fn rewrite_to_temporary_file(&self, existing_file: Option<&mut File>, dst: &mut let mut buffer = Vec::with_capacity(HISTORY_OUTPUT_BUFFER_SIZE + 128); for item in items { append_history_item_to_buffer(&item, &mut buffer); - if let Err(e) = flush_to_fd(&mut buffer, dst.as_raw_fd(), HISTORY_OUTPUT_BUFFER_SIZE) { + if let Err(e) = flush_to_file(&mut buffer, dst, HISTORY_OUTPUT_BUFFER_SIZE) { err = Some(e); break; } @@ -650,7 +654,7 @@ fn rewrite_to_temporary_file(&self, existing_file: Option<&mut File>, dst: &mut false } else { - flush_to_fd(&mut buffer, dst.as_raw_fd(), 0).is_ok() + flush_to_file(&mut buffer, dst, 0).is_ok() } } @@ -868,7 +872,7 @@ fn save_internal_via_appending(&mut self) -> bool { } } - if let Some(history_file) = history_file { + if let Some(mut history_file) = history_file { // We (hopefully successfully) 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 @@ -897,11 +901,7 @@ fn save_internal_via_appending(&mut self) -> bool { let item = &self.new_items[self.first_unwritten_new_item_index]; if item.should_write_to_disk() { append_history_item_to_buffer(item, &mut buffer); - res = flush_to_fd( - &mut buffer, - history_file.as_raw_fd(), - HISTORY_OUTPUT_BUFFER_SIZE, - ); + res = flush_to_file(&mut buffer, &mut history_file, HISTORY_OUTPUT_BUFFER_SIZE); if res.is_err() { break; } @@ -911,7 +911,7 @@ fn save_internal_via_appending(&mut self) -> bool { } if res.is_ok() { - res = flush_to_fd(&mut buffer, history_file.as_raw_fd(), 0); + res = flush_to_file(&mut buffer, &mut history_file, 0); } // Since we just modified the file, update our history_file_id to match its current state