Python
/
uv
mirror of https://github.com/astral-sh/uv
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix async windows file persist retries (#11008) 

The previous two versions of the code were bugged and would always
produce None when you retried (producing a hard LostState error).
This commit is contained in:
Aria Desires 2025-01-27 18:51:36 -05:00 committed by GitHub
parent c1a2ef12d2
commit a2db48d649
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 35 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
crates/uv-fs/src/lib.rs
View File

@ -310,22 +310,45 @@ pub async fn persist_with_retry(
// See: <https://github.com/astral-sh/uv/issues/1491> & <https://github.com/astral-sh/uv/issues/9531>
let to = to.as_ref();
// the `NamedTempFile` `persist` method consumes `self`, and returns it back inside the Error in case of `PersistError`
// Ok there's a lot of complex ownership stuff going on here.
//
// the `NamedTempFile` `persist` method consumes `self`, and returns it back inside
// the Error in case of `PersistError`:
// https://docs.rs/tempfile/latest/tempfile/struct.NamedTempFile.html#method.persist
// So we will update the `from` optional value in safe and borrow-checker friendly way every retry
// Allows us to use the NamedTempFile inside a FnMut closure used for backoff::retry
let mut from = Some(from);
let persist = move || {
// Needed because we cannot move out of `from`, a captured variable in an `FnMut` closure, and then pass it to the async move block
let mut from: Option<NamedTempFile> = from.take();
// So every time we fail, we need to reset the `NamedTempFile` to try again.
//
// Every time we (re)try we call this outer closure (`let persist = ...`), so it needs to
// be at least a `FnMut` (as opposed to `Fnonce`). However the closure needs to return a
// totally owned `Future` (so effectively it returns a `FnOnce`).
//
// But if the `Future` is totally owned it *necessarily* can't write back the `NamedTempFile`
// to somewhere the outer `FnMut` can see using references. So we need to use `Arc`s
// with interior mutability (`Mutex`) to have the closure and all the Futures it creates share
// a single memory location that the `NamedTempFile` can be shuttled in and out of.
//
// In spite of the Mutex all of this code will run logically serially, so there shouldn't be a
// chance for a race where we try to get the `NamedTempFile` but it's actually None. The code
// is just written pedantically/robustly.
let from = std::sync::Arc::new(std::sync::Mutex::new(Some(from)));
let persist = || {
// Turn our by-ref-captured Arc into an owned Arc that the Future can capture by-value
let from2 = from.clone();
async move {
if let Some(file) = from.take() {
let maybe_file: Option<NamedTempFile> = from2
.lock()
.map_err(|_| PersistRetryError::LostState)?
.take();
if let Some(file) = maybe_file {
file.persist(to).map_err(|err| {
let error_message = err.to_string();
// Set back the NamedTempFile returned back by the Error
from = Some(err.file);
PersistRetryError::Persist(error_message)
let error_message: String = err.to_string();
// Set back the `NamedTempFile` returned back by the Error
if let Ok(mut guard) = from2.lock() {
*guard = Some(err.file);
PersistRetryError::Persist(error_message)
} else {
PersistRetryError::LostState
}
})
} else {
Err(PersistRetryError::LostState)