more complicated locking

Author: ranger-ross

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

@@ -277,7 +277,7 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
         self.layout(unit.kind).build_dir().fingerprint(&dir)
     }
 
-    pub fn build_unit_lock(&self, unit: &Unit) -> PathBuf {
+    pub fn build_unit_lock(&self, unit: &Unit) -> (PathBuf, PathBuf) {
         let dir = self.pkg_dir(unit);
         self.layout(unit.kind).build_dir().build_unit_lock(&dir)
     }

src/cargo/core/compiler/layout.rs

@@ -348,8 +348,11 @@ impl BuildDirLayout {
             self.build().join(pkg_dir)
         }
     }
-    pub fn build_unit_lock(&self, pkg_dir: &str) -> PathBuf {
-        self.build_unit(pkg_dir).join("lock")
+    pub fn build_unit_lock(&self, pkg_dir: &str) -> (PathBuf, PathBuf) {
+        (
+            self.build_unit(pkg_dir).join("primary-lock"),
+            self.build_unit(pkg_dir).join("secondary-lock"),
+        )
     }
     /// Fetch the artifact path.
     pub fn artifact(&self) -> &Path {

src/cargo/core/compiler/locking.rs

@@ -1,6 +1,6 @@
 use std::{
     fs::{File, OpenOptions},
-    path::PathBuf,
+    path::{Path, PathBuf},
 };
 
 use itertools::Itertools;
@@ -9,59 +9,120 @@ use crate::core::compiler::{BuildRunner, Unit};
 
 pub struct CompilationLock {
     /// The path to the lock file of the unit to compile
-    unit: PathBuf,
+    unit: UnitLock,
     /// The paths to lock files of the unit's dependencies
-    dependency_units: Vec<PathBuf>,
+    dependency_units: Vec<UnitLock>,
 }
 
 impl CompilationLock {
     pub fn new(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> Self {
-        let unit_path = build_runner.files().build_unit_lock(unit);
+        let unit_lock = build_runner.files().build_unit_lock(unit).into();
 
         let dependency_units = build_runner
             .unit_deps(unit)
             .into_iter()
-            .map(|unit| build_runner.files().build_unit_lock(&unit.unit))
+            .map(|unit| build_runner.files().build_unit_lock(&unit.unit).into())
             .collect_vec();
 
         Self {
-            unit: unit_path,
+            unit: unit_lock,
             dependency_units,
         }
     }
 
-    pub fn lock(self) -> CompilationLockGuard {
-        let unit_lock = OpenOptions::new()
-            .create(true)
-            .write(true)
-            .append(true)
-            .open(self.unit)
-            .unwrap();
-        unit_lock.lock().unwrap();
+    pub fn lock(self) -> Self {
+        let unit_lock = self.unit.lock_exclusive();
 
         let dependency_locks = self
             .dependency_units
             .into_iter()
-            .map(|d| {
-                let f = OpenOptions::new()
-                    .create(true)
-                    .write(true)
-                    .append(true)
-                    .open(d)
-                    .unwrap();
-                f.lock_shared().unwrap();
-                f
-            })
+            .map(|d| d.lock_shared())
             .collect::<Vec<_>>();
 
-        CompilationLockGuard {
-            _lock: unit_lock,
-            _dependency_locks: dependency_locks,
+        CompilationLock {
+            unit: unit_lock,
+            dependency_units: dependency_locks,
         }
     }
+
+    pub fn rmeta_produced(&self) {
+        // Downgrade the lock on the unit we are building so that we can unblock other units to
+        // compile. We do not need to downgrade our dependency locks since they should always be a
+        // shared lock.
+        self.unit.downgrade();
+    }
+}
+
+struct UnitLock {
+    primary: PathBuf,
+    secondary: PathBuf,
+    gaurd: Option<UnitLockGuard>,
 }
 
-pub struct CompilationLockGuard {
-    _lock: File,
-    _dependency_locks: Vec<File>,
+struct UnitLockGuard {
+    primary: File,
+    _secondary: Option<File>,
+}
+
+impl UnitLock {
+    pub fn lock_exclusive(self) -> UnitLock {
+        let primary_lock = file_lock(&self.primary);
+        primary_lock.lock().unwrap();
+
+        let secondary_lock = file_lock(&self.secondary);
+        secondary_lock.lock().unwrap();
+
+        UnitLock {
+            primary: self.primary,
+            secondary: self.secondary,
+            gaurd: Some(UnitLockGuard {
+                primary: primary_lock,
+                _secondary: Some(secondary_lock),
+            }),
+        }
+    }
+
+    pub fn lock_shared(self) -> UnitLock {
+        let primary_lock = file_lock(&self.primary);
+        primary_lock.lock_shared().unwrap();
+
+        UnitLock {
+            primary: self.primary,
+            secondary: self.secondary,
+            gaurd: Some(UnitLockGuard {
+                primary: primary_lock,
+                _secondary: None,
+            }),
+        }
+    }
+
+    pub fn downgrade(&self) {
+        let gaurd = self.gaurd.as_ref().unwrap();
+
+        // TODO: Add debug asserts to verify the lock state?
+
+        // We know we have an exclusive lock here so we should never block.
+        // This is not super well documented but `lock_shared()` should downgrade the lock.
+        // TODO: Need to validate on other platforms...
+        gaurd.primary.lock_shared().unwrap();
+    }
+}
+
+fn file_lock<T: AsRef<Path>>(f: T) -> File {
+    OpenOptions::new()
+        .create(true)
+        .write(true)
+        .append(true)
+        .open(f)
+        .unwrap()
+}
+
+impl From<(PathBuf, PathBuf)> for UnitLock {
+    fn from(value: (PathBuf, PathBuf)) -> Self {
+        Self {
+            primary: value.0,
+            secondary: value.1,
+            gaurd: None,
+        }
+    }
 }

src/cargo/core/compiler/mod.rs

@@ -361,7 +361,7 @@ fn rustc(
     };
 
     return Ok(Work::new(move |state| {
-        let _guard = lock.map(|v| v.lock());
+        let mut lock = lock.map(|v| v.lock());
 
         // Artifacts are in a different location than typical units,
         // hence we must assure the crate- and target-dependent
@@ -445,6 +445,7 @@ fn rustc(
                             &manifest,
                             &target,
                             &mut output_options,
+                            lock.as_mut(),
                         )
                     },
                 )
@@ -1021,6 +1022,7 @@ fn rustdoc(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResult<W
                         &manifest,
                         &target,
                         &mut output_options,
+                        None,
                     )
                 },
                 false,
@@ -2008,8 +2010,9 @@ fn on_stderr_line(
     manifest: &ManifestErrorContext,
     target: &Target,
     options: &mut OutputOptions,
+    lock: Option<&mut CompilationLock>,
 ) -> CargoResult<()> {
-    if on_stderr_line_inner(state, line, package_id, manifest, target, options)? {
+    if on_stderr_line_inner(state, line, package_id, manifest, target, options, lock)? {
         // Check if caching is enabled.
         if let Some((path, cell)) = &mut options.cache_cell {
             // Cache the output, which will be replayed later when Fresh.
@@ -2030,6 +2033,7 @@ fn on_stderr_line_inner(
     manifest: &ManifestErrorContext,
     target: &Target,
     options: &mut OutputOptions,
+    lock: Option<&mut CompilationLock>,
 ) -> CargoResult<bool> {
     // We primarily want to use this function to process JSON messages from
     // rustc. The compiler should always print one JSON message per line, and
@@ -2265,6 +2269,10 @@ fn on_stderr_line_inner(
         trace!("found directive from rustc: `{}`", artifact.artifact);
         if artifact.artifact.ends_with(".rmeta") {
             debug!("looks like metadata finished early!");
+            if let Some(lock) = lock {
+                println!("rmeta produced! downgrading lock!");
+                lock.rmeta_produced();
+            }
             state.rmeta_produced();
         }
         return Ok(false);
@@ -2480,7 +2488,15 @@ fn replay_output_cache(
                 break;
             }
             let trimmed = line.trim_end_matches(&['\n', '\r'][..]);
-            on_stderr_line(state, trimmed, package_id, &manifest, &target, &mut options)?;
+            on_stderr_line(
+                state,
+                trimmed,
+                package_id,
+                &manifest,
+                &target,
+                &mut options,
+                None,
+            )?;
             line.clear();
         }
         Ok(())