fix: a race condition generating unique filenames for concurrent file operations (#3494)

This commit is contained in:
三咲雅 misaki masa 2026-01-03 16:40:49 +08:00 committed by GitHub
parent a6fcbb04df
commit 81c66a645f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 138 additions and 40 deletions

View file

@ -20,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/):
### Fixed
- `ya pkg` fails to write `package.toml` when the config directory does not exist ([#3482])
- A race condition generating unique filenames for concurrent file operations ([#3494])
## [v25.12.29]
@ -1578,3 +1579,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/):
[#3467]: https://github.com/sxyazi/yazi/pull/3467
[#3477]: https://github.com/sxyazi/yazi/pull/3477
[#3482]: https://github.com/sxyazi/yazi/pull/3482
[#3494]: https://github.com/sxyazi/yazi/pull/3494

View file

@ -76,6 +76,10 @@ pub trait Provider: Sized {
}
}
fn create_new(&self) -> impl Future<Output = io::Result<Self::File>> {
async move { self.gate().write(true).create_new(true).open(self.url()).await }
}
fn gate(&self) -> Self::Gate { Self::Gate::default() }
fn hard_link<P>(&self, to: P) -> impl Future<Output = io::Result<()>>

View file

@ -1,15 +1,15 @@
use std::{hash::{BuildHasher, Hash, Hasher}, mem};
use std::mem;
use anyhow::{Context, Result, anyhow};
use tokio::{io::{self, ErrorKind::NotFound}, sync::mpsc};
use tracing::warn;
use yazi_config::YAZI;
use yazi_fs::{Cwd, FsHash128, FsUrl, cha::Cha, ok_or_not_found, path::path_relative_to, provider::{Attrs, FileHolder, Provider, local::Local}};
use yazi_shared::{path::PathCow, timestamp_us, url::{AsUrl, UrlBuf, UrlCow, UrlLike}};
use yazi_vfs::{VfsCha, maybe_exists, must_be_dir, provider::{self, DirEntry}, unique_name};
use yazi_shared::{path::PathCow, url::{AsUrl, UrlCow, UrlLike}};
use yazi_vfs::{VfsCha, maybe_exists, provider::{self, DirEntry}, unique_file};
use super::{FileInCopy, FileInDelete, FileInHardlink, FileInLink, FileInTrash};
use crate::{LOW, NORMAL, TaskIn, TaskOp, TaskOps, ctx, file::{FileInCut, FileInDownload, FileInUpload, FileOutCopy, FileOutCopyDo, FileOutCut, FileOutCutDo, FileOutDelete, FileOutDeleteDo, FileOutDownload, FileOutDownloadDo, FileOutHardlink, FileOutHardlinkDo, FileOutLink, FileOutTrash, FileOutUpload, FileOutUploadDo}, hook::{HookInOutCopy, HookInOutCut}, ok_or_not_found, progress_or_break};
use crate::{LOW, NORMAL, TaskIn, TaskOp, TaskOps, ctx, file::{FileInCut, FileInDownload, FileInUpload, FileOutCopy, FileOutCopyDo, FileOutCut, FileOutCutDo, FileOutDelete, FileOutDeleteDo, FileOutDownload, FileOutDownloadDo, FileOutHardlink, FileOutHardlinkDo, FileOutLink, FileOutTrash, FileOutUpload, FileOutUploadDo, Transaction, Traverse}, hook::{HookInOutCopy, HookInOutCut}, ok_or_not_found, progress_or_break};
pub(crate) struct File {
ops: TaskOps,
@ -28,7 +28,7 @@ impl File {
let id = task.id;
if !task.force {
task.to = unique_name(task.to, must_be_dir(&task.from))
task.to = unique_file(mem::take(&mut task.to), task.init().await?.is_dir())
.await
.context("Cannot determine unique destination name")?;
}
@ -59,7 +59,7 @@ impl File {
}
pub(crate) async fn copy_do(&self, mut task: FileInCopy) -> Result<(), FileOutCopyDo> {
ok_or_not_found!(task, provider::remove_file(&task.to).await);
ok_or_not_found!(task, Transaction::unlink(&task.to).await);
let mut it =
ctx!(task, provider::copy_with_progress(&task.from, &task.to, task.cha.unwrap()).await)?;
@ -91,7 +91,7 @@ impl File {
let id = task.id;
if !task.force {
task.to = unique_name(mem::take(&mut task.to), must_be_dir(&task.from))
task.to = unique_file(mem::take(&mut task.to), task.init().await?.is_dir())
.await
.context("Cannot determine unique destination name")?;
}
@ -149,7 +149,7 @@ impl File {
}
pub(crate) async fn cut_do(&self, mut task: FileInCut) -> Result<(), FileOutCutDo> {
ok_or_not_found!(task, provider::remove_file(&task.to).await);
ok_or_not_found!(task, Transaction::unlink(&task.to).await);
let mut it =
ctx!(task, provider::copy_with_progress(&task.from, &task.to, task.cha.unwrap()).await)?;
@ -182,9 +182,8 @@ impl File {
pub(crate) async fn link(&self, mut task: FileInLink) -> Result<(), FileOutLink> {
if !task.force {
task.to = unique_name(task.to, must_be_dir(&task.from))
.await
.context("Cannot determine unique destination name")?;
task.to =
unique_file(task.to, false).await.context("Cannot determine unique destination name")?;
}
Ok(self.queue(task, NORMAL))
@ -229,9 +228,8 @@ impl File {
let id = task.id;
if !task.force {
task.to = unique_name(task.to, must_be_dir(&task.from))
.await
.context("Cannot determine unique destination name")?;
task.to =
unique_file(task.to, false).await.context("Cannot determine unique destination name")?;
}
super::traverse::<FileOutHardlink, _, _, _, _, _>(
@ -338,7 +336,7 @@ impl File {
let cha = task.cha.unwrap();
let cache = ctx!(task, task.url.cache(), "Cannot determine cache path")?;
let cache_tmp = ctx!(task, Self::tmp(&cache).await, "Cannot determine download cache")?;
let cache_tmp = ctx!(task, Transaction::tmp(&cache).await, "Cannot determine download cache")?;
let mut it = ctx!(task, provider::copy_with_progress(&task.url, &cache_tmp, cha).await)?;
loop {
@ -413,7 +411,8 @@ impl File {
Err(anyhow!("Failed to work on: {task:?}: remote file has changed since last download"))?;
}
let tmp = ctx!(task, Self::tmp(&task.url).await, "Cannot determine temporary upload path")?;
let tmp =
ctx!(task, Transaction::tmp(&task.url).await, "Cannot determine temporary upload path")?;
let mut it = ctx!(
task,
provider::copy_with_progress(cache, &tmp, Attrs {
@ -461,22 +460,6 @@ impl File {
};
Ok(if follow { Cha::from_follow(url, cha).await } else { cha })
}
async fn tmp<U>(url: U) -> io::Result<UrlBuf>
where
U: AsUrl,
{
let url = url.as_url();
let Some(parent) = url.parent() else {
Err(io::Error::new(io::ErrorKind::InvalidInput, "Url has no parent"))?
};
let mut h = foldhash::fast::FixedState::default().build_hasher();
url.hash(&mut h);
timestamp_us().hash(&mut h);
unique_name(parent.try_join(format!(".{:x}.%tmp", h.finish()))?, async { false }).await
}
}
impl File {

View file

@ -1 +1 @@
yazi_macro::mod_flat!(file out progress r#in traverse);
yazi_macro::mod_flat!(file out progress r#in transaction traverse);

View file

@ -0,0 +1,40 @@
use std::{hash::{BuildHasher, Hash, Hasher}, io};
use yazi_macro::ok_or_not_found;
use yazi_shared::{timestamp_us, url::{AsUrl, Url, UrlBuf}};
use yazi_vfs::{provider, unique_file};
pub(super) struct Transaction;
impl Transaction {
pub(super) async fn tmp<U>(url: U) -> io::Result<UrlBuf>
where
U: AsUrl,
{
Self::tmp_impl(url.as_url()).await
}
async fn tmp_impl(url: Url<'_>) -> io::Result<UrlBuf> {
let Some(parent) = url.parent() else {
Err(io::Error::new(io::ErrorKind::InvalidInput, "Url has no parent"))?
};
let mut h = foldhash::fast::FixedState::default().build_hasher();
url.hash(&mut h);
timestamp_us().hash(&mut h);
unique_file(parent.try_join(format!(".{:x}.%tmp", h.finish()))?, false).await
}
pub(super) async fn unlink<U>(url: U) -> io::Result<()>
where
U: AsUrl,
{
let url = url.as_url();
if ok_or_not_found!(provider::symlink_metadata(url).await, return Ok(())).is_link() {
provider::rename(Self::tmp(url).await?, url).await?;
}
Ok(())
}
}

View file

@ -6,7 +6,7 @@ use yazi_vfs::provider::{self};
use crate::{ctx, file::{FileInCopy, FileInCut, FileInDelete, FileInDownload, FileInHardlink, FileInUpload}};
trait Traverse {
pub(super) trait Traverse {
fn cha(&mut self) -> &mut Option<Cha>;
fn follow(&self) -> bool;

View file

@ -16,7 +16,7 @@ macro_rules! ok_or_not_found {
match $result {
Ok(v) => v,
Err(e) if e.kind() == std::io::ErrorKind::NotFound => $not_found,
Err(e) => ctx!($task, Err(e))?,
Err(e) => $crate::ctx!($task, Err(e))?,
}
};
($task:ident, $result:expr) => {

View file

@ -13,11 +13,7 @@ pub async fn maybe_exists(url: impl AsUrl) -> bool {
}
}
#[inline]
pub async fn must_be_dir(url: impl AsUrl) -> bool {
provider::metadata(url).await.is_ok_and(|m| m.is_dir())
}
// TODO: deprecate
pub async fn unique_name<F>(u: UrlBuf, append: F) -> io::Result<UrlBuf>
where
F: Future<Output = bool>,
@ -63,3 +59,62 @@ async fn _unique_name(mut url: UrlBuf, append: bool) -> io::Result<UrlBuf> {
Ok(url)
}
pub async fn unique_file(u: UrlBuf, is_dir: bool) -> io::Result<UrlBuf> {
let result = if is_dir {
provider::create_dir(&u).await
} else {
provider::create_new(&u).await.map(|_| ())
};
match result {
Ok(()) => Ok(u),
Err(e) if e.kind() == io::ErrorKind::AlreadyExists => _unique_file(u, is_dir).await,
Err(e) => Err(e),
}
}
async fn _unique_file(mut url: UrlBuf, is_dir: bool) -> io::Result<UrlBuf> {
let Some(stem) = url.stem().map(|s| s.to_owned()) else {
return Err(io::Error::new(io::ErrorKind::InvalidInput, "empty file stem"));
};
let dot_ext = match url.ext() {
Some(e) => {
let mut s = StrandBuf::with_capacity(url.kind(), e.len() + 1);
s.push_str(".");
s.try_push(e)?;
s
}
None => StrandBuf::default(),
};
let mut name = StrandBuf::with_capacity(url.kind(), stem.len() + dot_ext.len() + 5);
for i in 1u64.. {
name.clear();
name.try_push(&stem)?;
if is_dir {
name.try_push(&dot_ext)?;
name.push_str(format!("_{i}"));
} else {
name.push_str(format!("_{i}"));
name.try_push(&dot_ext)?;
}
url.try_set_name(&name)?;
let result = if is_dir {
provider::create_dir(&url).await
} else {
provider::create_new(&url).await.map(|_| ())
};
match result {
Ok(()) => break,
Err(e) if e.kind() == io::ErrorKind::AlreadyExists => {}
Err(e) => Err(e)?,
};
}
Ok(url)
}

View file

@ -106,6 +106,13 @@ where
Providers::new(url.as_url()).await?.create_dir_all().await
}
pub async fn create_new<U>(url: U) -> io::Result<RwFile>
where
U: AsUrl,
{
Providers::new(url.as_url()).await?.create_new().await
}
pub async fn hard_link<U, V>(original: U, link: V) -> io::Result<()>
where
U: AsUrl,

View file

@ -87,6 +87,13 @@ impl<'a> Provider for Providers<'a> {
}
}
async fn create_new(&self) -> io::Result<Self::File> {
Ok(match self {
Self::Local(p) => p.create_new().await?.into(),
Self::Sftp(p) => p.create_new().await?.into(),
})
}
async fn hard_link<P>(&self, to: P) -> io::Result<()>
where
P: AsPath,