1
Fork 0
mirror of https://github.com/RGBCube/uutils-coreutils synced 2025-07-28 11:37:44 +00:00

sort: prevent race when deleting files

Move the creation of temporary files into next_file so that it happens
while the lock is taken.

Previously, only the computation of the new file path happened while
the lock was taken, while the creation happened later.
This commit is contained in:
Michael Debertol 2021-08-07 23:00:42 +02:00
parent 3a6769f74b
commit 21043d3605
3 changed files with 17 additions and 15 deletions

View file

@ -12,6 +12,7 @@
//! The buffers for the individual chunks are recycled. There are two buffers. //! The buffers for the individual chunks are recycled. There are two buffers.
use std::cmp::Ordering; use std::cmp::Ordering;
use std::fs::File;
use std::io::Write; use std::io::Write;
use std::path::PathBuf; use std::path::PathBuf;
use std::{ use std::{
@ -238,7 +239,7 @@ fn read_write_loop<I: WriteableTmpFile>(
let tmp_file = write::<I>( let tmp_file = write::<I>(
&mut chunk, &mut chunk,
tmp_dir.next_file_path()?, tmp_dir.next_file()?,
settings.compress_prog.as_deref(), settings.compress_prog.as_deref(),
separator, separator,
)?; )?;
@ -268,7 +269,7 @@ fn read_write_loop<I: WriteableTmpFile>(
/// `compress_prog` is used to optionally compress file contents. /// `compress_prog` is used to optionally compress file contents.
fn write<I: WriteableTmpFile>( fn write<I: WriteableTmpFile>(
chunk: &mut Chunk, chunk: &mut Chunk,
file: PathBuf, file: (File, PathBuf),
compress_prog: Option<&str>, compress_prog: Option<&str>,
separator: u8, separator: u8,
) -> UResult<I::Closed> { ) -> UResult<I::Closed> {

View file

@ -46,7 +46,7 @@ fn replace_output_file_in_input_files(
if let Some(copy) = &copy { if let Some(copy) = &copy {
*file = copy.clone().into_os_string(); *file = copy.clone().into_os_string();
} else { } else {
let copy_path = tmp_dir.next_file_path()?; let (_file, copy_path) = tmp_dir.next_file()?;
std::fs::copy(file_path, &copy_path) std::fs::copy(file_path, &copy_path)
.map_err(|error| SortError::OpenTmpFileFailed { error })?; .map_err(|error| SortError::OpenTmpFileFailed { error })?;
*file = copy_path.clone().into_os_string(); *file = copy_path.clone().into_os_string();
@ -110,7 +110,7 @@ pub fn merge_with_file_limit<
remaining_files = remaining_files.saturating_sub(settings.merge_batch_size); remaining_files = remaining_files.saturating_sub(settings.merge_batch_size);
let merger = merge_without_limit(batches.next().unwrap(), settings)?; let merger = merge_without_limit(batches.next().unwrap(), settings)?;
let mut tmp_file = let mut tmp_file =
Tmp::create(tmp_dir.next_file_path()?, settings.compress_prog.as_deref())?; Tmp::create(tmp_dir.next_file()?, settings.compress_prog.as_deref())?;
merger.write_all_to(settings, tmp_file.as_write())?; merger.write_all_to(settings, tmp_file.as_write())?;
temporary_files.push(tmp_file.finished_writing()?); temporary_files.push(tmp_file.finished_writing()?);
} }
@ -379,7 +379,7 @@ fn check_child_success(mut child: Child, program: &str) -> UResult<()> {
pub trait WriteableTmpFile: Sized { pub trait WriteableTmpFile: Sized {
type Closed: ClosedTmpFile; type Closed: ClosedTmpFile;
type InnerWrite: Write; type InnerWrite: Write;
fn create(path: PathBuf, compress_prog: Option<&str>) -> UResult<Self>; fn create(file: (File, PathBuf), compress_prog: Option<&str>) -> UResult<Self>;
/// Closes the temporary file. /// Closes the temporary file.
fn finished_writing(self) -> UResult<Self::Closed>; fn finished_writing(self) -> UResult<Self::Closed>;
fn as_write(&mut self) -> &mut Self::InnerWrite; fn as_write(&mut self) -> &mut Self::InnerWrite;
@ -414,11 +414,9 @@ impl WriteableTmpFile for WriteablePlainTmpFile {
type Closed = ClosedPlainTmpFile; type Closed = ClosedPlainTmpFile;
type InnerWrite = BufWriter<File>; type InnerWrite = BufWriter<File>;
fn create(path: PathBuf, _: Option<&str>) -> UResult<Self> { fn create((file, path): (File, PathBuf), _: Option<&str>) -> UResult<Self> {
Ok(WriteablePlainTmpFile { Ok(WriteablePlainTmpFile {
file: BufWriter::new( file: BufWriter::new(file),
File::create(&path).map_err(|error| SortError::OpenTmpFileFailed { error })?,
),
path, path,
}) })
} }
@ -476,12 +474,10 @@ impl WriteableTmpFile for WriteableCompressedTmpFile {
type Closed = ClosedCompressedTmpFile; type Closed = ClosedCompressedTmpFile;
type InnerWrite = BufWriter<ChildStdin>; type InnerWrite = BufWriter<ChildStdin>;
fn create(path: PathBuf, compress_prog: Option<&str>) -> UResult<Self> { fn create((file, path): (File, PathBuf), compress_prog: Option<&str>) -> UResult<Self> {
let compress_prog = compress_prog.unwrap(); let compress_prog = compress_prog.unwrap();
let mut command = Command::new(compress_prog); let mut command = Command::new(compress_prog);
let tmp_file = command.stdin(Stdio::piped()).stdout(file);
File::create(&path).map_err(|error| SortError::OpenTmpFileFailed { error })?;
command.stdin(Stdio::piped()).stdout(tmp_file);
let mut child = command let mut child = command
.spawn() .spawn()
.map_err(|err| SortError::CompressProgExecutionFailed { .map_err(|err| SortError::CompressProgExecutionFailed {

View file

@ -1,4 +1,5 @@
use std::{ use std::{
fs::File,
path::{Path, PathBuf}, path::{Path, PathBuf},
sync::{Arc, Mutex}, sync::{Arc, Mutex},
}; };
@ -54,7 +55,7 @@ impl TmpDirWrapper {
.map_err(|e| USimpleError::new(2, format!("failed to set up signal handler: {}", e))) .map_err(|e| USimpleError::new(2, format!("failed to set up signal handler: {}", e)))
} }
pub fn next_file_path(&mut self) -> UResult<PathBuf> { pub fn next_file(&mut self) -> UResult<(File, PathBuf)> {
if self.temp_dir.is_none() { if self.temp_dir.is_none() {
self.init_tmp_dir()?; self.init_tmp_dir()?;
} }
@ -62,7 +63,11 @@ impl TmpDirWrapper {
let _lock = self.lock.lock().unwrap(); let _lock = self.lock.lock().unwrap();
let file_name = self.size.to_string(); let file_name = self.size.to_string();
self.size += 1; self.size += 1;
Ok(self.temp_dir.as_ref().unwrap().path().join(file_name)) let path = self.temp_dir.as_ref().unwrap().path().join(file_name);
Ok((
File::create(&path).map_err(|error| SortError::OpenTmpFileFailed { error })?,
path,
))
} }
} }