refactor: Remove lazycell

Fixes #9310

Author: epage

Cargo.lock

@@ -63,7 +63,6 @@ indexmap = "2.12.0"
 itertools = "0.14.0"
 jiff = { version = "0.2.15", default-features = false, features = [ "std" ] }
 jobserver = "0.1.34"
-lazycell = "1.3.0"
 libc = "0.2.177"
 libgit2-sys = "0.18.2"
 libloading = "0.8.9"
@@ -185,7 +184,6 @@ indexmap.workspace = true
 itertools.workspace = true
 jiff = { workspace = true, features = ["serde", "std"] }
 jobserver.workspace = true
-lazycell.workspace = true
 libgit2-sys.workspace = true
 memchr.workspace = true
 opener.workspace = true

Cargo.toml

@@ -1,18 +1,18 @@
 //! See [`CompilationFiles`].
 
+use std::cell::OnceCell;
 use std::collections::HashMap;
 use std::fmt;
 use std::hash::{Hash, Hasher};
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
-use lazycell::LazyCell;
 use tracing::debug;
 
 use super::{BuildContext, BuildRunner, CompileKind, FileFlavor, Layout};
 use crate::core::compiler::{CompileMode, CompileTarget, CrateType, FileType, Unit};
 use crate::core::{Target, TargetKind, Workspace};
-use crate::util::{self, CargoResult, StableHasher};
+use crate::util::{self, CargoResult, OnceExt, StableHasher};
 
 /// This is a generic version number that can be changed to make
 /// backwards-incompatible changes to any file structures in the output
@@ -128,7 +128,7 @@ pub struct CompilationFiles<'a, 'gctx> {
     /// Metadata hash to use for each unit.
     metas: HashMap<Unit, Metadata>,
     /// For each Unit, a list all files produced.
-    outputs: HashMap<Unit, LazyCell<Arc<Vec<OutputFile>>>>,
+    outputs: HashMap<Unit, OnceCell<Arc<Vec<OutputFile>>>>,
 }
 
 /// Info about a single file emitted by the compiler.
@@ -168,7 +168,7 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
         let outputs = metas
             .keys()
             .cloned()
-            .map(|unit| (unit, LazyCell::new()))
+            .map(|unit| (unit, OnceCell::new()))
             .collect();
         CompilationFiles {
             ws: build_runner.bcx.ws,

src/cargo/core/compiler/build_runner/compilation_files.rs

@@ -52,6 +52,7 @@ pub mod unit_dependencies;
 pub mod unit_graph;
 
 use std::borrow::Cow;
+use std::cell::OnceCell;
 use std::collections::{BTreeMap, HashMap, HashSet};
 use std::env;
 use std::ffi::{OsStr, OsString};
@@ -66,7 +67,6 @@ use annotate_snippets::{AnnotationKind, Group, Level, Renderer, Snippet};
 use anyhow::{Context as _, Error};
 use cargo_platform::{Cfg, Platform};
 use itertools::Itertools;
-use lazycell::LazyCell;
 use regex::Regex;
 use tracing::{debug, instrument, trace};
 
@@ -95,6 +95,7 @@ pub use crate::core::compiler::unit::{Unit, UnitInterner};
 use crate::core::manifest::TargetSourcePath;
 use crate::core::profiles::{PanicStrategy, Profile, StripInner};
 use crate::core::{Feature, PackageId, Target, Verbosity};
+use crate::util::OnceExt;
 use crate::util::context::WarningHandling;
 use crate::util::errors::{CargoResult, VerboseError};
 use crate::util::interning::InternedString;
@@ -1897,7 +1898,7 @@ struct OutputOptions {
     /// is fresh. The file is created lazily so that in the normal case, lots
     /// of empty files are not created. If this is None, the output will not
     /// be cached (such as when replaying cached messages).
-    cache_cell: Option<(PathBuf, LazyCell<File>)>,
+    cache_cell: Option<(PathBuf, OnceCell<File>)>,
     /// If `true`, display any diagnostics.
     /// Other types of JSON messages are processed regardless
     /// of the value of this flag.
@@ -1917,7 +1918,7 @@ impl OutputOptions {
         let path = build_runner.files().message_cache_path(unit);
         // Remove old cache, ignore ENOENT, which is the common case.
         drop(fs::remove_file(&path));
-        let cache_cell = Some((path, LazyCell::new()));
+        let cache_cell = Some((path, OnceCell::new()));
         let show_diagnostics =
             build_runner.bcx.gctx.warning_handling().unwrap_or_default() != WarningHandling::Allow;
         OutputOptions {

src/cargo/core/compiler/mod.rs

@@ -1,3 +1,4 @@
+use std::cell::OnceCell;
 use std::cell::{Cell, Ref, RefCell, RefMut};
 use std::cmp::Ordering;
 use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
@@ -12,7 +13,6 @@ use anyhow::Context as _;
 use cargo_util_schemas::manifest::{Hints, RustVersion};
 use curl::easy::Easy;
 use curl::multi::{EasyHandle, Multi};
-use lazycell::LazyCell;
 use semver::Version;
 use serde::Serialize;
 use tracing::debug;
@@ -287,7 +287,7 @@ impl hash::Hash for Package {
 /// This is primarily used to convert a set of `PackageId`s to `Package`s. It
 /// will download as needed, or used the cached download if available.
 pub struct PackageSet<'gctx> {
-    packages: HashMap<PackageId, LazyCell<Package>>,
+    packages: HashMap<PackageId, OnceCell<Package>>,
     sources: RefCell<SourceMap<'gctx>>,
     gctx: &'gctx GlobalContext,
     multi: Multi,
@@ -408,7 +408,7 @@ impl<'gctx> PackageSet<'gctx> {
         Ok(PackageSet {
             packages: package_ids
                 .iter()
-                .map(|&id| (id, LazyCell::new()))
+                .map(|&id| (id, OnceCell::new()))
                 .collect(),
             sources: RefCell::new(sources),
             gctx,
@@ -423,7 +423,7 @@ impl<'gctx> PackageSet<'gctx> {
     }
 
     pub fn packages(&self) -> impl Iterator<Item = &Package> {
-        self.packages.values().filter_map(|p| p.borrow())
+        self.packages.values().filter_map(|p| p.get())
     }
 
     pub fn enable_download<'a>(&'a self) -> CargoResult<Downloads<'a, 'gctx>> {
@@ -457,7 +457,7 @@ impl<'gctx> PackageSet<'gctx> {
     }
 
     pub fn get_one(&self, id: PackageId) -> CargoResult<&Package> {
-        if let Some(pkg) = self.packages.get(&id).and_then(|slot| slot.borrow()) {
+        if let Some(pkg) = self.packages.get(&id).and_then(|slot| slot.get()) {
             return Ok(pkg);
         }
         Ok(self.get_many(Some(id))?.remove(0))
@@ -700,7 +700,7 @@ impl<'a, 'gctx> Downloads<'a, 'gctx> {
             .packages
             .get(&id)
             .ok_or_else(|| internal(format!("couldn't find `{}` in package set", id)))?;
-        if let Some(pkg) = slot.borrow() {
+        if let Some(pkg) = slot.get() {
             return Ok(Some(pkg));
         }
 
@@ -717,8 +717,8 @@ impl<'a, 'gctx> Downloads<'a, 'gctx> {
         let (url, descriptor, authorization) = match pkg {
             MaybePackage::Ready(pkg) => {
                 debug!("{} doesn't need a download", id);
-                assert!(slot.fill(pkg).is_ok());
-                return Ok(Some(slot.borrow().unwrap()));
+                assert!(slot.set(pkg).is_ok());
+                return Ok(Some(slot.get().unwrap()));
             }
             MaybePackage::Download {
                 url,
@@ -941,8 +941,8 @@ impl<'a, 'gctx> Downloads<'a, 'gctx> {
             .set(self.next_speed_check.get() + finish_dur);
 
         let slot = &self.set.packages[&dl.id];
-        assert!(slot.fill(pkg).is_ok());
-        Ok(slot.borrow().unwrap())
+        assert!(slot.set(pkg).is_ok());
+        Ok(slot.get().unwrap())
     }
 
     fn enqueue(&mut self, dl: Download<'gctx>, handle: Easy) -> CargoResult<()> {

src/cargo/core/package.rs

@@ -11,10 +11,10 @@ use crate::sources::registry::{LoadResponse, RegistryConfig, RegistryData};
 use crate::util::cache_lock::CacheLockMode;
 use crate::util::errors::CargoResult;
 use crate::util::interning::InternedString;
-use crate::util::{Filesystem, GlobalContext};
+use crate::util::{Filesystem, GlobalContext, OnceExt};
 use anyhow::Context as _;
 use cargo_util::paths;
-use lazycell::LazyCell;
+use std::cell::OnceCell;
 use std::cell::{Cell, Ref, RefCell};
 use std::fs::File;
 use std::mem;
@@ -71,7 +71,7 @@ pub struct RemoteRegistry<'gctx> {
     /// [tree object]: https://git-scm.com/book/en/v2/Git-Internals-Git-Objects#_tree_objects
     tree: RefCell<Option<git2::Tree<'static>>>,
     /// A Git repository that contains the actual index we want.
-    repo: LazyCell<git2::Repository>,
+    repo: OnceCell<git2::Repository>,
     /// The current HEAD commit of the underlying Git repository.
     head: Cell<Option<git2::Oid>>,
     /// This stores sha value of the current HEAD commit for convenience.
@@ -103,7 +103,7 @@ impl<'gctx> RemoteRegistry<'gctx> {
             gctx,
             index_git_ref: GitReference::DefaultBranch,
             tree: RefCell::new(None),
-            repo: LazyCell::new(),
+            repo: OnceCell::new(),
             head: Cell::new(None),
             current_sha: Cell::new(None),
             needs_update: false,
@@ -378,7 +378,7 @@ impl<'gctx> RegistryData for RemoteRegistry<'gctx> {
         // Fetch the latest version of our `index_git_ref` into the index
         // checkout.
         let url = self.source_id.url();
-        let repo = self.repo.borrow_mut().unwrap();
+        let repo = self.repo.get_mut().unwrap();
         git::fetch(
             repo,
             url.as_str(),

src/cargo/sources/registry/remote.rs

@@ -10,6 +10,11 @@ pub trait OnceExt {
     where
         F: FnOnce() -> Result<Self::T, E>;
 
+    /// This might run `f` multiple times if different threads start initializing at once.
+    fn try_borrow_mut_with<F, E>(&mut self, f: F) -> Result<&mut Self::T, E>
+    where
+        F: FnOnce() -> Result<Self::T, E>;
+
     fn replace(&mut self, new_value: Self::T) -> Option<Self::T>;
 
     fn filled(&self) -> bool;
@@ -37,6 +42,25 @@ impl<T> OnceExt for std::sync::OnceLock<T> {
         Ok(self.get().unwrap())
     }
 
+    fn try_borrow_mut_with<F, E>(&mut self, f: F) -> Result<&mut T, E>
+    where
+        F: FnOnce() -> Result<T, E>,
+    {
+        let value = if let Some(value) = self.take() {
+            value
+        } else {
+            // This is not how the unstable `OnceLock::get_or_try_init` works. That only starts `f` if
+            // no other `f` is executing and the value is not initialized. However, correctly implementing that is
+            // hard (one has properly handle panics in `f`) and not doable with the stable API of `OnceLock`.
+            f()?
+        };
+        // Another thread might have initialized `self` since we checked that `self.get()` returns `None`. If this is the case, `self.set()`
+        // returns an error. We ignore it and return the value set by the other
+        // thread.
+        let _ = self.set(value);
+        Ok(self.get_mut().unwrap())
+    }
+
     fn replace(&mut self, new_value: T) -> Option<T> {
         if let Some(value) = self.get_mut() {
             Some(std::mem::replace(value, new_value))
@@ -74,6 +98,25 @@ impl<T> OnceExt for std::cell::OnceCell<T> {
         Ok(self.get().unwrap())
     }
 
+    fn try_borrow_mut_with<F, E>(&mut self, f: F) -> Result<&mut T, E>
+    where
+        F: FnOnce() -> Result<T, E>,
+    {
+        let value = if let Some(value) = self.take() {
+            value
+        } else {
+            // This is not how the unstable `OnceLock::get_or_try_init` works. That only starts `f` if
+            // no other `f` is executing and the value is not initialized. However, correctly implementing that is
+            // hard (one has properly handle panics in `f`) and not doable with the stable API of `OnceLock`.
+            f()?
+        };
+        // Another thread might have initialized `self` since we checked that `self.get()` returns `None`. If this is the case, `self.set()`
+        // returns an error. We ignore it and return the value set by the other
+        // thread.
+        let _ = self.set(value);
+        Ok(self.get_mut().unwrap())
+    }
+
     fn replace(&mut self, new_value: T) -> Option<T> {
         if let Some(value) = self.get_mut() {
             Some(std::mem::replace(value, new_value))

src/cargo/util/once.rs

@@ -1,5 +1,6 @@
 use annotate_snippets::{AnnotationKind, Group, Level, Snippet};
 use std::borrow::Cow;
+use std::cell::OnceCell;
 use std::collections::{BTreeMap, BTreeSet, HashMap};
 use std::ffi::OsStr;
 use std::path::{Path, PathBuf};
@@ -17,7 +18,6 @@ use cargo_util_schemas::manifest::{
 };
 use cargo_util_schemas::manifest::{RustVersion, StringOrBool};
 use itertools::Itertools;
-use lazycell::LazyCell;
 use pathdiff::diff_paths;
 use url::Url;
 
@@ -33,7 +33,9 @@ use crate::sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY};
 use crate::util::errors::{CargoResult, ManifestError};
 use crate::util::interning::InternedString;
 use crate::util::lints::{get_key_value_span, rel_cwd_manifest_path};
-use crate::util::{self, GlobalContext, IntoUrl, OptVersionReq, context::ConfigRelativePath};
+use crate::util::{
+    self, GlobalContext, IntoUrl, OnceExt, OptVersionReq, context::ConfigRelativePath,
+};
 
 mod embedded;
 mod targets;
@@ -299,7 +301,7 @@ fn normalize_toml(
 ) -> CargoResult<manifest::TomlManifest> {
     let package_root = manifest_file.parent().unwrap();
 
-    let inherit_cell: LazyCell<InheritableFields> = LazyCell::new();
+    let inherit_cell: OnceCell<InheritableFields> = OnceCell::new();
     let inherit = || {
         inherit_cell
             .try_borrow_with(|| load_inheritable_fields(gctx, manifest_file, &workspace_config))