Skip to content

Commit 5b36e7c

Browse files
committed
feat(locking): Added reference counted file locking
1 parent 2c04739 commit 5b36e7c

File tree

1 file changed

+187
-25
lines changed

1 file changed

+187
-25
lines changed

src/cargo/core/compiler/locking.rs

Lines changed: 187 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,15 @@
3131
//! [`CompilationLock`] is the primary interface for locking.
3232
3333
use std::{
34-
collections::HashSet,
34+
collections::{HashMap, HashSet},
3535
fs::{File, OpenOptions},
3636
path::{Path, PathBuf},
37+
sync::{Arc, Condvar, LazyLock, Mutex},
3738
};
3839

39-
use anyhow::Context;
40+
use anyhow::{Context, anyhow};
4041
use itertools::Itertools;
41-
use tracing::{instrument, trace};
42+
use tracing::{instrument, trace, warn};
4243

4344
use crate::{
4445
CargoResult,
@@ -122,8 +123,17 @@ struct UnitLock {
122123
}
123124

124125
struct UnitLockGuard {
125-
partial: File,
126-
_full: Option<File>,
126+
partial: Arc<RcFileLock>,
127+
full: Option<Arc<RcFileLock>>,
128+
}
129+
130+
impl Drop for UnitLockGuard {
131+
fn drop(&mut self) {
132+
self.partial.unlock().unwrap();
133+
if let Some(full) = &self.full {
134+
full.unlock().unwrap();
135+
}
136+
}
127137
}
128138

129139
impl UnitLock {
@@ -138,37 +148,48 @@ impl UnitLock {
138148
pub fn lock_exclusive(&mut self) -> CargoResult<()> {
139149
assert!(self.guard.is_none());
140150

141-
let partial = open_file(&self.partial)?;
151+
let partial = FileLockInterner::get_or_create_lock(&self.partial)?;
142152
partial.lock()?;
143153

144-
let full = open_file(&self.full)?;
145-
full.lock()?;
154+
let unlock_partial = || {
155+
if let Err(err) = partial.unlock() {
156+
warn!("Failed to unlock partial lock after failing to take full lock {err}");
157+
}
158+
};
159+
160+
let full =
161+
FileLockInterner::get_or_create_lock(&self.full).inspect_err(|_| unlock_partial())?;
162+
full.lock().inspect_err(|_| unlock_partial())?;
146163

147164
self.guard = Some(UnitLockGuard {
148165
partial,
149-
_full: Some(full),
166+
full: Some(full),
150167
});
151168
Ok(())
152169
}
153170

154171
pub fn lock_shared(&mut self, ty: &SharedLockType) -> CargoResult<()> {
155172
assert!(self.guard.is_none());
156173

157-
let partial = open_file(&self.partial)?;
174+
let partial = FileLockInterner::get_or_create_lock(&self.partial)?;
158175
partial.lock_shared()?;
159176

177+
let unlock_partial = || {
178+
if let Err(err) = partial.unlock() {
179+
warn!("Failed to unlock partial lock after failing to take full lock {err}");
180+
}
181+
};
182+
160183
let full = if matches!(ty, SharedLockType::Full) {
161-
let full_lock = open_file(&self.full)?;
162-
full_lock.lock_shared()?;
184+
let full_lock = FileLockInterner::get_or_create_lock(&self.full)
185+
.inspect_err(|_| unlock_partial())?;
186+
full_lock.lock_shared().inspect_err(|_| unlock_partial())?;
163187
Some(full_lock)
164188
} else {
165189
None
166190
};
167191

168-
self.guard = Some(UnitLockGuard {
169-
partial,
170-
_full: full,
171-
});
192+
self.guard = Some(UnitLockGuard { partial, full });
172193
Ok(())
173194
}
174195

@@ -177,15 +198,7 @@ impl UnitLock {
177198
.guard
178199
.as_ref()
179200
.context("guard was None while calling downgrade")?;
180-
181-
// NOTE:
182-
// > Subsequent flock() calls on an already locked file will convert an existing lock to the new lock mode.
183-
// https://man7.org/linux/man-pages/man2/flock.2.html
184-
//
185-
// However, the `std::file::File::lock/lock_shared` is allowed to change this in the
186-
// future. So its probably up to us if we are okay with using this or if we want to use a
187-
// different interface to flock.
188-
guard.partial.lock_shared()?;
201+
guard.partial.downgrade()?;
189202

190203
Ok(())
191204
}
@@ -220,3 +233,152 @@ fn all_dependency_units<'a>(
220233
inner(build_runner, unit, &mut results);
221234
return results;
222235
}
236+
237+
/// An interner to manage [`RcFileLock`]s to make sharing across compilation jobs easier.
238+
pub struct FileLockInterner {
239+
locks: Mutex<HashMap<PathBuf, Arc<RcFileLock>>>,
240+
}
241+
242+
impl FileLockInterner {
243+
pub fn new() -> Self {
244+
Self {
245+
locks: Mutex::new(HashMap::new()),
246+
}
247+
}
248+
249+
pub fn get_or_create_lock(path: &Path) -> CargoResult<Arc<RcFileLock>> {
250+
static GLOBAL: LazyLock<FileLockInterner> = LazyLock::new(FileLockInterner::new);
251+
252+
let mut locks = GLOBAL
253+
.locks
254+
.lock()
255+
.map_err(|_| anyhow!("lock was poisoned"))?;
256+
257+
if let Some(lock) = locks.get(path) {
258+
return Ok(Arc::clone(lock));
259+
}
260+
261+
let file = open_file(&path)?;
262+
263+
let lock = Arc::new(RcFileLock {
264+
inner: Mutex::new(RcFileLockInner {
265+
file,
266+
share_count: 0,
267+
exclusive: false,
268+
}),
269+
condvar: Condvar::new(),
270+
});
271+
272+
locks.insert(path.to_path_buf(), Arc::clone(&lock));
273+
274+
return Ok(lock);
275+
}
276+
}
277+
278+
/// A reference counted file lock.
279+
///
280+
/// This lock is designed to reduce file descriptors by sharing a single file descriptor for a
281+
/// given lock when the lock is shared. The motivation for this is to avoid hitting file descriptor
282+
/// limits when fine grain locking is enabled.
283+
pub struct RcFileLock {
284+
inner: Mutex<RcFileLockInner>,
285+
condvar: Condvar,
286+
}
287+
288+
struct RcFileLockInner {
289+
file: File,
290+
exclusive: bool,
291+
share_count: u32,
292+
}
293+
294+
impl RcFileLock {
295+
pub fn lock(&self) -> CargoResult<()> {
296+
let mut inner = self
297+
.inner
298+
.lock()
299+
.map_err(|_| anyhow!("lock was poisoned"))?;
300+
301+
while inner.exclusive || inner.share_count > 0 {
302+
inner = self
303+
.condvar
304+
.wait(inner)
305+
.map_err(|_| anyhow!("lock was poisoned"))?;
306+
}
307+
308+
inner.file.lock()?;
309+
inner.exclusive = true;
310+
311+
Ok(())
312+
}
313+
314+
pub fn lock_shared(&self) -> CargoResult<()> {
315+
let mut inner = self
316+
.inner
317+
.lock()
318+
.map_err(|_| anyhow!("lock was poisoned"))?;
319+
320+
while inner.exclusive {
321+
inner = self
322+
.condvar
323+
.wait(inner)
324+
.map_err(|_| anyhow!("lock was poisoned"))?;
325+
}
326+
327+
if inner.share_count == 0 {
328+
inner.file.lock_shared()?;
329+
inner.share_count = 1;
330+
} else {
331+
inner.share_count += 1;
332+
}
333+
334+
Ok(())
335+
}
336+
337+
pub fn unlock(&self) -> CargoResult<()> {
338+
let mut inner = self
339+
.inner
340+
.lock()
341+
.map_err(|_| anyhow!("lock was poisoned"))?;
342+
343+
if inner.exclusive {
344+
assert!(inner.share_count == 0);
345+
inner.file.unlock()?;
346+
self.condvar.notify_all();
347+
inner.exclusive = false;
348+
} else {
349+
if inner.share_count > 1 {
350+
inner.share_count -= 1;
351+
} else {
352+
inner.file.unlock()?;
353+
inner.share_count = 0;
354+
self.condvar.notify_all();
355+
}
356+
}
357+
358+
Ok(())
359+
}
360+
361+
pub fn downgrade(&self) -> CargoResult<()> {
362+
let mut inner = self
363+
.inner
364+
.lock()
365+
.map_err(|_| anyhow!("lock was poisoned"))?;
366+
367+
assert!(inner.exclusive);
368+
assert!(inner.share_count == 0);
369+
370+
// NOTE:
371+
// > Subsequent flock() calls on an already locked file will convert an existing lock to the new lock mode.
372+
// https://man7.org/linux/man-pages/man2/flock.2.html
373+
//
374+
// However, the `std::file::File::lock/lock_shared` is allowed to change this in the
375+
// future. So its probably up to us if we are okay with using this or if we want to use a
376+
// different interface to flock.
377+
inner.file.lock_shared()?;
378+
379+
inner.exclusive = false;
380+
inner.share_count = 1;
381+
382+
Ok(())
383+
}
384+
}

0 commit comments

Comments
 (0)