Visitar URL original
Evict corrupt cached Python archives by zaniebot · Pull Request #191 · astral-sh/uv-dev · GitHub
Skip to content

Evict corrupt cached Python archives - #191

Draft
zaniebot wants to merge 4 commits into
mainfrom
zb/bughunt200/119-corrupt-python-cache
Draft

zaniebot wants to merge 4 commits into
mainfrom
zb/bughunt200/119-corrupt-python-cache

Conversation

@zaniebot

@zaniebot zaniebot commented Jul 16, 2026 •

Copy link
Copy Markdown
Collaborator

A downloaded Python archive is cached before its hash is verified, so a corrupt archive is reused on later runs. Remove it after a hash mismatch so the next run can download it again. This does not retry immediately or remove unrelated failed downloads. uv#19795 made cached Python downloads survive pruning, so corrupt entries can persist.

@astral-automations-bot
astral-automations-bot Bot force-pushed the zb/bughunt200/119-corrupt-python-cache branch from f565224 to b059d80 Compare September 1, 2026 21:00
@zaniebot zaniebot added priority:2 Important: bounded correctness, release reliability, committed work, or proven performance gains. size:2 Moderate: one subsystem with meaningful invariants, edge cases, or operational evidence to inspect. bug Something isn't working risk:3 Credible risk of meaningful incorrect behavior or regression; bounded or recoverable. labels Sep 9, 2026
@astral-automations-bot
astral-automations-bot Bot force-pushed the zb/bughunt200/119-corrupt-python-cache branch from b059d80 to 31475c2 Compare October 6, 2026 00:28
@codspeed

codspeed Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

⚠️ 8 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

✅ 69 untouched benchmarks
⏩ 35 skipped benchmarks1


Comparing zb/bughunt200/119-corrupt-python-cache (aa4191d) with main (f1c34ca)

Open in CodSpeed

Footnotes

  1. 35 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@astral-automations-bot
astral-automations-bot Bot force-pushed the zb/bughunt200/119-corrupt-python-cache branch from 31475c2 to d5d2405 Compare October 8, 2026 03:54
Comment on lines +623 to +624
if let Err(Error::HashMismatch { .. }) = &result {
match fs_err::tokio::remove_file(&target_cache_file).await {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Coordinate eviction with shared-cache writers

On Unix, installs with different installation directories can share UV_PYTHON_CACHE_DIR; the callers' installation-directory locks do not serialize them. Two readers can open the corrupt archive, one can remove it, and a subsequent attempt can publish a valid replacement before the slower reader reaches this removal. That reader then deletes the replacement, potentially causing the downloader's subsequent File::open to fail with NotFound. Use a cache-entry lock covering opening, publication, validation, and eviction so removal targets the archive that failed verification.

match fs_err::tokio::remove_file(&target_cache_file).await {
Ok(()) => {}
Err(err) if err.kind() == io::ErrorKind::NotFound => {}
Err(err) => return Err(err.into()),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve the hash mismatch when eviction fails

If the cached archive is readable but cannot be deleted, such as in a read-only shared cache, this return replaces the confirmed HashMismatch with the cleanup I/O error. Callers consequently lose the integrity-failure diagnostic and its expected/computed hashes. Keep eviction best-effort and report its failure separately while returning the original mismatch, or retain both errors in a contextual error.

@zaniebot
zaniebot deployed to automations October 8, 2026 23:57 — with GitHub Actions Active
@zaniebot
zaniebot deployed to automations October 9, 2026 00:02 — with GitHub Actions Active
Comment on lines +1393 to +1395
let temp_dir = tempfile::tempdir().expect("temporary directory should be created");
let cache_dir = temp_dir.path().join("cache");
fs_err::create_dir_all(&cache_dir).expect("cache directory should be created");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] Exercise a cache hit in the entry-lock test

This fixture creates the cache directory and lock, but never writes ababababa-python.zip. It therefore exercises a cache miss: a refactor that locks only downloads while leaving cached readers unlocked would still pass, despite breaking eviction coordination. Seed the matching cached archive before acquiring the lock so this test verifies that existing-archive reads wait too.

Comment on lines +1317 to +1322
fs_err::write(
&cached_archive,
[
0x50, 0x4b, 0x03, 0x04, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x4d, 0x1e, 0x48, 0x5d,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0d, 0x00,
0x00, 0x00, 0x70, 0x79, 0x74, 0x68, 0x6f, 0x6e, 0x2f, 0x70, 0x79, 0x74, 0x68, 0x6f,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] Make the archive fixture's contents explicit

These bytes encode a valid ZIP containing an empty python/python file. Successful extraction is essential here: malformed archive data would return ExtractError before reaching hash validation. The opaque byte array hides that requirement and makes fixture changes require understanding ZIP encoding. Prefer constructing named archive entries with the existing uv_test::archive::write_tar_gz helper in the python_install.rs integration module, and document why the expected digest deliberately differs.

@zaniebot
zaniebot deployed to automations October 9, 2026 03:35 — with GitHub Actions Active
@astral-automations-bot
astral-automations-bot Bot force-pushed the zb/bughunt200/119-corrupt-python-cache branch from d0e207a to 68dfb81 Compare October 9, 2026 20:01
@astral-automations-bot
astral-automations-bot Bot force-pushed the zb/bughunt200/119-corrupt-python-cache branch from 68dfb81 to 4dd00dc Compare October 14, 2026 18:37
Comment on lines +4314 to +4318
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
fs_err::set_permissions(cache.path(), std::fs::Permissions::from_mode(0o555))?;
let (snapshot, _) = uv_test::run_and_format(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] Separate the read-only cache scenario into a named test

This Unix-only block checks retention when lock acquisition fails, while the invocation below checks eviction after successful locking. These are independent regression scenarios: a failure here prevents the eviction assertion from running on Unix, and the read-only behavior cannot be selected independently. Following the repository’s test guidance, move this scenario into a separate #[cfg(unix)] test with explicit fixture setup, leaving the writable-cache eviction test cross-platform.

@astral-automations-bot
astral-automations-bot Bot force-pushed the zb/bughunt200/119-corrupt-python-cache branch from 4dd00dc to aa4191d Compare October 16, 2026 18:12

This branch was successfully deployed

1 active deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority:2 Important: bounded correctness, release reliability, committed work, or proven performance gains. risk:3 Credible risk of meaningful incorrect behavior or regression; bounded or recoverable. size:2 Moderate: one subsystem with meaningful invariants, edge cases, or operational evidence to inspect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants