Stop leaking strings in Python downloads (#15418)

We should not unnecessarily leak memory. Instead, we follow the general
patterns and use `Cow` for strings that can be from either a static or a
dynamic source.
This commit is contained in:
konsti 2025-08-21 17:54:39 +02:00 committed by GitHub
parent 0397595e53
commit 25bedeadea
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 34 additions and 29 deletions

View file

@ -97,7 +97,7 @@ pub enum Error {
#[error("No download found for request: {}", _0.green())] #[error("No download found for request: {}", _0.green())]
NoDownloadFound(PythonDownloadRequest), NoDownloadFound(PythonDownloadRequest),
#[error("A mirror was provided via `{0}`, but the URL does not match the expected format: {0}")] #[error("A mirror was provided via `{0}`, but the URL does not match the expected format: {0}")]
Mirror(&'static str, &'static str), Mirror(&'static str, String),
#[error("Failed to determine the libc used on the current platform")] #[error("Failed to determine the libc used on the current platform")]
LibcDetection(#[from] platform::LibcDetectionError), LibcDetection(#[from] platform::LibcDetectionError),
#[error("Remote Python downloads JSON is not yet supported, please use a local path")] #[error("Remote Python downloads JSON is not yet supported, please use a local path")]
@ -142,8 +142,8 @@ impl Error {
#[derive(Debug, PartialEq, Eq, Clone, Hash)] #[derive(Debug, PartialEq, Eq, Clone, Hash)]
pub struct ManagedPythonDownload { pub struct ManagedPythonDownload {
key: PythonInstallationKey, key: PythonInstallationKey,
url: &'static str, url: Cow<'static, str>,
sha256: Option<&'static str>, sha256: Option<Cow<'static, str>>,
} }
#[derive(Debug, Clone, Default, Eq, PartialEq, Hash)] #[derive(Debug, Clone, Default, Eq, PartialEq, Hash)]
@ -836,8 +836,8 @@ impl ManagedPythonDownload {
Ok(downloads.iter()) Ok(downloads.iter())
} }
pub fn url(&self) -> &'static str { pub fn url(&self) -> &Cow<'static, str> {
self.url &self.url
} }
pub fn key(&self) -> &PythonInstallationKey { pub fn key(&self) -> &PythonInstallationKey {
@ -848,8 +848,8 @@ impl ManagedPythonDownload {
self.key.os() self.key.os()
} }
pub fn sha256(&self) -> Option<&'static str> { pub fn sha256(&self) -> Option<&Cow<'static, str>> {
self.sha256 self.sha256.as_ref()
} }
/// Download and extract a Python distribution, retrying on failure. /// Download and extract a Python distribution, retrying on failure.
@ -963,7 +963,7 @@ impl ManagedPythonDownload {
{ {
let python_builds_dir = PathBuf::from(python_builds_dir); let python_builds_dir = PathBuf::from(python_builds_dir);
fs_err::create_dir_all(&python_builds_dir)?; fs_err::create_dir_all(&python_builds_dir)?;
let hash_prefix = match self.sha256 { let hash_prefix = match self.sha256.as_deref() {
Some(sha) => { Some(sha) => {
// Shorten the hash to avoid too-long-filename errors // Shorten the hash to avoid too-long-filename errors
&sha[..9] &sha[..9]
@ -1169,11 +1169,11 @@ impl ManagedPythonDownload {
reporter: Option<&dyn Reporter>, reporter: Option<&dyn Reporter>,
direction: Direction, direction: Direction,
) -> Result<(), Error> { ) -> Result<(), Error> {
let mut hashers = self let mut hashers = if self.sha256.is_some() {
.sha256 vec![Hasher::from(HashAlgorithm::Sha256)]
.into_iter() } else {
.map(|_| Hasher::from(HashAlgorithm::Sha256)) vec![]
.collect::<Vec<_>>(); };
let mut hasher = uv_extract::hash::HashReader::new(reader, &mut hashers); let mut hasher = uv_extract::hash::HashReader::new(reader, &mut hashers);
if let Some(reporter) = reporter { if let Some(reporter) = reporter {
@ -1191,7 +1191,7 @@ impl ManagedPythonDownload {
hasher.finish().await.map_err(Error::HashExhaustion)?; hasher.finish().await.map_err(Error::HashExhaustion)?;
// Check the hash // Check the hash
if let Some(expected) = self.sha256 { if let Some(expected) = self.sha256.as_deref() {
let actual = HashDigest::from(hashers.pop().unwrap()).digest; let actual = HashDigest::from(hashers.pop().unwrap()).digest;
if !actual.eq_ignore_ascii_case(expected) { if !actual.eq_ignore_ascii_case(expected) {
return Err(Error::HashMismatch { return Err(Error::HashMismatch {
@ -1222,7 +1222,10 @@ impl ManagedPythonDownload {
let Some(suffix) = self.url.strip_prefix( let Some(suffix) = self.url.strip_prefix(
"https://github.com/astral-sh/python-build-standalone/releases/download/", "https://github.com/astral-sh/python-build-standalone/releases/download/",
) else { ) else {
return Err(Error::Mirror(EnvVars::UV_PYTHON_INSTALL_MIRROR, self.url)); return Err(Error::Mirror(
EnvVars::UV_PYTHON_INSTALL_MIRROR,
self.url.to_string(),
));
}; };
return Ok(Url::parse( return Ok(Url::parse(
format!("{}/{}", mirror.trim_end_matches('/'), suffix).as_str(), format!("{}/{}", mirror.trim_end_matches('/'), suffix).as_str(),
@ -1234,7 +1237,10 @@ impl ManagedPythonDownload {
if let Some(mirror) = pypy_install_mirror { if let Some(mirror) = pypy_install_mirror {
let Some(suffix) = self.url.strip_prefix("https://downloads.python.org/pypy/") let Some(suffix) = self.url.strip_prefix("https://downloads.python.org/pypy/")
else { else {
return Err(Error::Mirror(EnvVars::UV_PYPY_INSTALL_MIRROR, self.url)); return Err(Error::Mirror(
EnvVars::UV_PYPY_INSTALL_MIRROR,
self.url.to_string(),
));
}; };
return Ok(Url::parse( return Ok(Url::parse(
format!("{}/{}", mirror.trim_end_matches('/'), suffix).as_str(), format!("{}/{}", mirror.trim_end_matches('/'), suffix).as_str(),
@ -1245,7 +1251,7 @@ impl ManagedPythonDownload {
_ => {} _ => {}
} }
Ok(Url::parse(self.url)?) Ok(Url::parse(&self.url)?)
} }
} }
@ -1337,10 +1343,8 @@ fn parse_json_downloads(
} }
}; };
let url = Box::leak(entry.url.into_boxed_str()) as &'static str; let url = Cow::Owned(entry.url);
let sha256 = entry let sha256 = entry.sha256.map(Cow::Owned);
.sha256
.map(|s| Box::leak(s.into_boxed_str()) as &'static str);
Some(ManagedPythonDownload { Some(ManagedPythonDownload {
key: PythonInstallationKey::new_from_version( key: PythonInstallationKey::new_from_version(

View file

@ -1,4 +1,5 @@
use core::fmt; use core::fmt;
use std::borrow::Cow;
use std::cmp::Reverse; use std::cmp::Reverse;
use std::ffi::OsStr; use std::ffi::OsStr;
use std::io::{self, Write}; use std::io::{self, Write};
@ -314,11 +315,11 @@ pub struct ManagedPythonInstallation {
/// The URL with the Python archive. /// The URL with the Python archive.
/// ///
/// Empty when self was constructed from a path. /// Empty when self was constructed from a path.
url: Option<&'static str>, url: Option<Cow<'static, str>>,
/// The SHA256 of the Python archive at the URL. /// The SHA256 of the Python archive at the URL.
/// ///
/// Empty when self was constructed from a path. /// Empty when self was constructed from a path.
sha256: Option<&'static str>, sha256: Option<Cow<'static, str>>,
} }
impl ManagedPythonInstallation { impl ManagedPythonInstallation {
@ -326,8 +327,8 @@ impl ManagedPythonInstallation {
Self { Self {
path, path,
key: download.key().clone(), key: download.key().clone(),
url: Some(download.url()), url: Some(download.url().clone()),
sha256: download.sha256(), sha256: download.sha256().cloned(),
} }
} }
@ -668,12 +669,12 @@ impl ManagedPythonInstallation {
self.key.patch != other.key.patch self.key.patch != other.key.patch
} }
pub fn url(&self) -> Option<&'static str> { pub fn url(&self) -> Option<&str> {
self.url self.url.as_deref()
} }
pub fn sha256(&self) -> Option<&'static str> { pub fn sha256(&self) -> Option<&str> {
self.sha256 self.sha256.as_deref()
} }
} }