diff --git a/CHANGELOG.md b/CHANGELOG.md index b98f3b34..59b8ecbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/yazi-fs/src/provider/traits.rs b/yazi-fs/src/provider/traits.rs index c8739819..28d38f3d 100644 --- a/yazi-fs/src/provider/traits.rs +++ b/yazi-fs/src/provider/traits.rs @@ -76,6 +76,10 @@ pub trait Provider: Sized { } } + fn create_new(&self) -> impl Future> { + async move { self.gate().write(true).create_new(true).open(self.url()).await } + } + fn gate(&self) -> Self::Gate { Self::Gate::default() } fn hard_link

(&self, to: P) -> impl Future> diff --git a/yazi-scheduler/src/file/file.rs b/yazi-scheduler/src/file/file.rs index 503d12f9..8c64c5a0 100644 --- a/yazi-scheduler/src/file/file.rs +++ b/yazi-scheduler/src/file/file.rs @@ -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::( @@ -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(url: U) -> io::Result - 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 { diff --git a/yazi-scheduler/src/file/mod.rs b/yazi-scheduler/src/file/mod.rs index 2a1a30ce..d4beaadd 100644 --- a/yazi-scheduler/src/file/mod.rs +++ b/yazi-scheduler/src/file/mod.rs @@ -1 +1 @@ -yazi_macro::mod_flat!(file out progress r#in traverse); +yazi_macro::mod_flat!(file out progress r#in transaction traverse); diff --git a/yazi-scheduler/src/file/transaction.rs b/yazi-scheduler/src/file/transaction.rs new file mode 100644 index 00000000..45abfa80 --- /dev/null +++ b/yazi-scheduler/src/file/transaction.rs @@ -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(url: U) -> io::Result + where + U: AsUrl, + { + Self::tmp_impl(url.as_url()).await + } + + async fn tmp_impl(url: Url<'_>) -> io::Result { + 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(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(()) + } +} diff --git a/yazi-scheduler/src/file/traverse.rs b/yazi-scheduler/src/file/traverse.rs index 6e367a67..345b5acc 100644 --- a/yazi-scheduler/src/file/traverse.rs +++ b/yazi-scheduler/src/file/traverse.rs @@ -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; fn follow(&self) -> bool; diff --git a/yazi-scheduler/src/macros.rs b/yazi-scheduler/src/macros.rs index ae8fb014..cc824b0f 100644 --- a/yazi-scheduler/src/macros.rs +++ b/yazi-scheduler/src/macros.rs @@ -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) => { diff --git a/yazi-vfs/src/fns.rs b/yazi-vfs/src/fns.rs index d83bd2ae..479d54d5 100644 --- a/yazi-vfs/src/fns.rs +++ b/yazi-vfs/src/fns.rs @@ -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(u: UrlBuf, append: F) -> io::Result where F: Future, @@ -63,3 +59,62 @@ async fn _unique_name(mut url: UrlBuf, append: bool) -> io::Result { Ok(url) } + +pub async fn unique_file(u: UrlBuf, is_dir: bool) -> io::Result { + 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 { + 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) +} diff --git a/yazi-vfs/src/provider/provider.rs b/yazi-vfs/src/provider/provider.rs index 35607d86..52d9a011 100644 --- a/yazi-vfs/src/provider/provider.rs +++ b/yazi-vfs/src/provider/provider.rs @@ -106,6 +106,13 @@ where Providers::new(url.as_url()).await?.create_dir_all().await } +pub async fn create_new(url: U) -> io::Result +where + U: AsUrl, +{ + Providers::new(url.as_url()).await?.create_new().await +} + pub async fn hard_link(original: U, link: V) -> io::Result<()> where U: AsUrl, diff --git a/yazi-vfs/src/provider/providers.rs b/yazi-vfs/src/provider/providers.rs index 7a6f8243..48aefbbc 100644 --- a/yazi-vfs/src/provider/providers.rs +++ b/yazi-vfs/src/provider/providers.rs @@ -87,6 +87,13 @@ impl<'a> Provider for Providers<'a> { } } + async fn create_new(&self) -> io::Result { + Ok(match self { + Self::Local(p) => p.create_new().await?.into(), + Self::Sftp(p) => p.create_new().await?.into(), + }) + } + async fn hard_link

(&self, to: P) -> io::Result<()> where P: AsPath,