caisin/typst
1
0
Fork 0
mirror of https://github.com/typst/typst synced 2025-08-14 23:18:32 +08:00
Code Issues Packages Projects Releases Wiki Activity

Fix race condition

Browse Source 
Because the package directory was created immediately empty (before extraction), other Typst instances would already start using it and complain about missing `typst.toml`. The package directory must not exist until the rename takes place.

To simplify that, this commit moves the location of the temporary dir into the package subfolder so that it's a move to a direct sibling location.
This commit is contained in:
Laurenz 2025-03-11 12:18:43 +01:00
parent 401c591a84
commit 3c6a284387
1 changed files with 16 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
crates/typst-kit/src/package.rs
View File

@ -102,8 +102,7 @@ impl PackageStorage {
// Download from network if it doesn't exist yet. // Download from network if it doesn't exist yet.
if spec.namespace == DEFAULT_NAMESPACE { if spec.namespace == DEFAULT_NAMESPACE {
let package_base_dir = cache_dir.join(&*spec.namespace); self.download_package(spec, cache_dir, progress)?;
self.download_package(&package_base_dir, spec, &dir, progress)?;
if dir.exists() { if dir.exists() {
return Ok(dir); return Ok(dir);
} }
@ -169,9 +168,8 @@ impl PackageStorage {
/// Panics if the package spec namespace isn't `DEFAULT_NAMESPACE`. /// Panics if the package spec namespace isn't `DEFAULT_NAMESPACE`.
pub fn download_package( pub fn download_package(
&self, &self,
package_base_dir: &Path,
spec: &PackageSpec, spec: &PackageSpec,
package_dir: &Path, cache_dir: &Path,
progress: &mut dyn Progress, progress: &mut dyn Progress,
) -> PackageResult<()> { ) -> PackageResult<()> {
assert_eq!(spec.namespace, DEFAULT_NAMESPACE); assert_eq!(spec.namespace, DEFAULT_NAMESPACE);
@ -198,22 +196,24 @@ impl PackageStorage {
// Temporary place where the package will be extracted before being // Temporary place where the package will be extracted before being
// moved to the target directory. To avoid name clashing we use a PRNG // moved to the target directory. To avoid name clashing we use a PRNG
// to get a unique directory name. // to get a unique directory name.
let tempdir = Tempdir::create( let tempdir = Tempdir::create(cache_dir.join(format!(
package_base_dir, "{}/{}/.tmp-{}-{}",
format_args!("{}-{}", spec.name, spec.version), spec.namespace,
) spec.name,
spec.version,
fastrand::u32(..),
)))
.map_err(|err| error("failed to create temporary package directory", err))?; .map_err(|err| error("failed to create temporary package directory", err))?;
// Create the package directory into which we'll move the extracted
// package.
std::fs::create_dir_all(package_dir)
.map_err(|err| error("failed to create package directory", err))?;
let decompressed = flate2::read::GzDecoder::new(data.as_slice()); let decompressed = flate2::read::GzDecoder::new(data.as_slice());
tar::Archive::new(decompressed) tar::Archive::new(decompressed)
.unpack(&tempdir) .unpack(&tempdir)
.map_err(|err| PackageError::MalformedArchive(Some(eco_format!("{err}"))))?; .map_err(|err| PackageError::MalformedArchive(Some(eco_format!("{err}"))))?;
// The place into which we'll move the extracted package.
let package_dir =
cache_dir.join(format!("{}/{}/{}", spec.namespace, spec.name, spec.version));
// As to not overcomplicate the code to combat an already rare case // As to not overcomplicate the code to combat an already rare case
// where multiple instances try to download the same package version // where multiple instances try to download the same package version
// concurrently, we are abusing the behavior of the `rename` FS // concurrently, we are abusing the behavior of the `rename` FS
@ -239,7 +239,7 @@ impl PackageStorage {
// (currently) justify the additional code complexity. If such situation // (currently) justify the additional code complexity. If such situation
// occur and it will be reported, then we probably would consider a // occur and it will be reported, then we probably would consider a
// more comprehensive solution (i.e., file locking or checksums). // more comprehensive solution (i.e., file locking or checksums).
match fs::rename(&tempdir, package_dir) { match fs::rename(&tempdir, &package_dir) {
Ok(()) => Ok(()), Ok(()) => Ok(()),
Err(err) if err.kind() == io::ErrorKind::DirectoryNotEmpty => Ok(()), Err(err) if err.kind() == io::ErrorKind::DirectoryNotEmpty => Ok(()),
Err(err) => { Err(err) => {
@ -261,10 +261,8 @@ struct MinimalPackageInfo {
struct Tempdir(PathBuf); struct Tempdir(PathBuf);
impl Tempdir { impl Tempdir {
/// Creates a temporary directory of the form /// Creates a directory at the path and auto-cleans it.
/// `.tmp-{inner}-{randomsuffix}` in `base`. fn create(path: PathBuf) -> io::Result<Self> {
fn create(base: &Path, inner: impl std::fmt::Display) -> io::Result<Self> {
let path = base.join(format!(".tmp-{inner}-{}", fastrand::u32(..)));
std::fs::create_dir_all(&path)?; std::fs::create_dir_all(&path)?;
Ok(Self(path)) Ok(Self(path))
} }