Skip to content

Commit 350f02c

Browse files
Merge pull request #796 from charlespierce/concurrent_install_2
[Medium] Extend Locks through entire install process
2 parents 39c85d9 + 6f78b98 commit 350f02c

File tree

9 files changed

+136
-29
lines changed

9 files changed

+136
-29
lines changed

crates/volta-core/src/error/kind.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,9 @@ pub enum ErrorKind {
171171
errors: Vec<String>,
172172
},
173173

174+
/// Thrown when unable to acquire a lock on the Volta directory
175+
LockAcquireError,
176+
174177
/// Thrown when BinConfig (read from file) does not contain Platform info.
175178
NoBinPlatform {
176179
binary: String,
@@ -826,7 +829,11 @@ To {action} the packages '{name}' and '{version}', please {action} them in separ
826829
name, call_to_action, formatted_errs
827830
)
828831
}
829-
832+
// Note: No CTA as this error is purely informational and shouldn't be exposed to the user
833+
ErrorKind::LockAcquireError => write!(
834+
f,
835+
"Unable to acquire lock on Volta directory"
836+
),
830837
ErrorKind::NoBinPlatform { binary } => write!(
831838
f,
832839
"Platform info for executable `{}` is missing
@@ -1477,6 +1484,7 @@ impl ErrorKind {
14771484
ErrorKind::InvalidHookOutput { .. } => ExitCode::ExecutionFailure,
14781485
ErrorKind::InvalidInvocation { .. } => ExitCode::InvalidArguments,
14791486
ErrorKind::InvalidToolName { .. } => ExitCode::InvalidArguments,
1487+
ErrorKind::LockAcquireError => ExitCode::FileSystemError,
14801488
ErrorKind::NoBinPlatform { .. } => ExitCode::ExecutionFailure,
14811489
ErrorKind::NoBundledNpm { .. } => ExitCode::ConfigurationError,
14821490
ErrorKind::NoCommandLineYarn => ExitCode::ConfigurationError,

crates/volta-core/src/shim.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,12 @@ use std::path::Path;
88
use crate::error::{Context, ErrorKind, Fallible, VoltaError};
99
use crate::fs::{read_dir_eager, symlink_file};
1010
use crate::layout::{volta_home, volta_install};
11+
use crate::sync::VoltaLock;
1112
use log::debug;
1213

1314
pub fn regenerate_shims_for_dir(dir: &Path) -> Fallible<()> {
15+
// Acquire a lock on the Volta directory, if possible, to prevent concurrent changes
16+
let _lock = VoltaLock::acquire();
1417
debug!("Rebuilding shims for directory: {}", dir.display());
1518
for shim_name in get_shim_list_deduped(dir)?.iter() {
1619
delete(shim_name)?;

crates/volta-core/src/sync.rs

Lines changed: 107 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,129 @@
1+
//! Inter-process locking on the Volta directory
2+
//!
3+
//! To avoid issues where multiple separate invocations of Volta modify the
4+
//! data directory simultaneously, we provide a locking mechanism that only
5+
//! allows a single process to modify the directory at a time.
6+
//!
7+
//! However, within a single process, we may attempt to lock the directory in
8+
//! different code paths. For example, when installing a package we require a
9+
//! lock, however we also may need to install Node, which requires a lock as
10+
//! well. To avoid deadlocks in those situations, we track the state of the
11+
//! lock globally:
12+
//!
13+
//! - If a lock is requested and no locks are active, then we acquire a file
14+
//! lock on the `volta.lock` file and initialize the state with a count of 1
15+
//! - If a lock already exists, then we increment the count of active locks
16+
//! - When a lock is no longer needed, we decrement the count of active locks
17+
//! - When the last lock is released, we release the file lock and clear the
18+
//! global lock state.
19+
//!
20+
//! This allows multiple code paths to request a lock and not worry about
21+
//! potential deadlocks, while still preventing multiple processes from making
22+
//! concurrent changes.
23+
124
use std::fs::{File, OpenOptions};
25+
use std::marker::PhantomData;
226
use std::ops::Drop;
3-
use std::path::Path;
27+
use std::sync::Mutex;
428

29+
use crate::error::{Context, ErrorKind, Fallible};
30+
use crate::layout::volta_home;
531
use crate::style::progress_spinner;
632
use fs2::FileExt;
33+
use lazy_static::lazy_static;
734
use log::debug;
835

36+
lazy_static! {
37+
static ref LOCK_STATE: Mutex<Option<LockState>> = Mutex::new(None);
38+
}
39+
40+
/// The current state of locks for this process.
41+
///
42+
/// Note: To ensure thread safety _within_ this process, we enclose the
43+
/// state in a Mutex. This Mutex and it's associated locks are separate
44+
/// from the overall process lock and are only used to ensure the count
45+
/// is accurately maintained within a given process.
46+
struct LockState {
47+
file: File,
48+
count: usize,
49+
}
50+
951
const LOCK_FILE: &str = "volta.lock";
1052

11-
/// An RAII implementation of an exclusive lock on the Volta directory. When this falls out of scope,
12-
/// the lock will be unlocked.
53+
/// An RAII implementation of a process lock on the Volta directory. A given Volta process can have
54+
/// multiple active locks, but only one process can have any locks at a time.
55+
///
56+
/// Once all of the `VoltaLock` objects go out of scope, the lock will be released to other
57+
/// processes.
1358
pub struct VoltaLock {
14-
inner: File,
59+
// Private field ensures that this cannot be created except for with the `acquire()` method
60+
_private: PhantomData<()>,
1561
}
1662

1763
impl VoltaLock {
18-
pub fn acquire(volta_home: &Path) -> std::io::Result<Self> {
19-
let path = volta_home.join(LOCK_FILE);
20-
debug!("Acquiring lock on Volta directory: {}", path.display());
21-
22-
let file = OpenOptions::new().write(true).create(true).open(path)?;
23-
// First we try to lock the file without blocking. If that fails, then we show a spinner
24-
// and block until the lock completes.
25-
if file.try_lock_exclusive().is_err() {
26-
let spinner = progress_spinner("Waiting for file lock on Volta directory");
27-
// Note: Blocks until the file can be locked
28-
let lock_result = file.lock_exclusive();
29-
spinner.finish_and_clear();
30-
lock_result?;
64+
pub fn acquire() -> Fallible<Self> {
65+
let mut state = LOCK_STATE
66+
.lock()
67+
.with_context(|| ErrorKind::LockAcquireError)?;
68+
69+
// Check if there is an active lock for this process. If so, increment
70+
// the count of active locks. If not, create a file lock and initialize
71+
// the state with a count of 1
72+
match &mut *state {
73+
Some(inner) => {
74+
inner.count += 1;
75+
}
76+
None => {
77+
let path = volta_home()?.root().join(LOCK_FILE);
78+
debug!("Acquiring lock on Volta directory: {}", path.display());
79+
80+
let file = OpenOptions::new()
81+
.write(true)
82+
.create(true)
83+
.open(path)
84+
.with_context(|| ErrorKind::LockAcquireError)?;
85+
// First we try to lock the file without blocking. If that fails, then we show a spinner
86+
// and block until the lock completes.
87+
if file.try_lock_exclusive().is_err() {
88+
let spinner = progress_spinner("Waiting for file lock on Volta directory");
89+
// Note: Blocks until the file can be locked
90+
let lock_result = file
91+
.lock_exclusive()
92+
.with_context(|| ErrorKind::LockAcquireError);
93+
spinner.finish_and_clear();
94+
lock_result?;
95+
}
96+
97+
*state = Some(LockState { file, count: 1 });
98+
}
3199
}
32100

33-
Ok(Self { inner: file })
101+
Ok(Self {
102+
_private: PhantomData,
103+
})
34104
}
35105
}
36106

37107
impl Drop for VoltaLock {
38-
#[allow(unused_must_use)]
39108
fn drop(&mut self) {
40-
self.inner.unlock();
109+
// On drop, decrement the count of active locks. If the count is 1,
110+
// then this is the last active lock, so instead unlock the file and
111+
// clear out the lock state.
112+
if let Ok(mut state) = LOCK_STATE.lock() {
113+
match &mut *state {
114+
Some(inner) => {
115+
if inner.count == 1 {
116+
debug!("Unlocking Volta Directory");
117+
let _ = inner.file.unlock();
118+
*state = None;
119+
} else {
120+
inner.count -= 1;
121+
}
122+
}
123+
None => {
124+
debug!("Unexpected unlock of Volta directory when it wasn't locked");
125+
}
126+
}
127+
}
41128
}
42129
}

crates/volta-core/src/tool/mod.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use std::fmt::{self, Display};
22

33
use crate::error::{ErrorKind, Fallible};
4-
use crate::layout::volta_home;
54
use crate::session::Session;
65
use crate::style::{note_prefix, success_prefix, tool_version};
76
use crate::sync::VoltaLock;
@@ -144,17 +143,17 @@ enum FetchStatus {
144143
/// Uses the supplied `already_fetched` predicate to determine if a tool is available or not.
145144
///
146145
/// This uses double-checking logic, to correctly handle concurrent fetch requests:
147-
/// If `already_fetched` indicates that a fetch is needed, we acquire an exclusive lock on the Volta directory
148-
/// Then, we check _again_, to confirm that no other process completed the fetch while we waited for the lock
146+
///
147+
/// - If `already_fetched` indicates that a fetch is needed, we acquire an exclusive lock on the Volta directory
148+
/// - Then, we check _again_, to confirm that no other process completed the fetch while we waited for the lock
149149
///
150150
/// Note: If acquiring the lock fails, we proceed anyway, since the fetch is still necessary.
151151
fn check_fetched<F>(already_fetched: F) -> Fallible<FetchStatus>
152152
where
153153
F: Fn() -> Fallible<bool>,
154154
{
155155
if !already_fetched()? {
156-
let home = volta_home()?.root();
157-
let lock = match VoltaLock::acquire(home) {
156+
let lock = match VoltaLock::acquire() {
158157
Ok(l) => Some(l),
159158
Err(_) => {
160159
debug!("Unable to acquire lock on Volta directory!");

crates/volta-core/src/tool/node/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use crate::error::{ErrorKind, Fallible};
88
use crate::inventory::node_available;
99
use crate::session::Session;
1010
use crate::style::{note_prefix, tool_version};
11+
use crate::sync::VoltaLock;
1112
use cfg_if::cfg_if;
1213
use log::info;
1314
use semver::Version;
@@ -130,6 +131,8 @@ impl Tool for Node {
130131
Ok(())
131132
}
132133
fn install(self: Box<Self>, session: &mut Session) -> Fallible<()> {
134+
// Acquire a lock on the Volta directory, if possible, to prevent concurrent changes
135+
let _lock = VoltaLock::acquire();
133136
let node_version = self.ensure_fetched(session)?;
134137

135138
let default_toolchain = session.toolchain_mut()?;

crates/volta-core/src/tool/npm/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use crate::error::{Context, ErrorKind, Fallible};
99
use crate::inventory::npm_available;
1010
use crate::session::Session;
1111
use crate::style::{success_prefix, tool_version};
12+
use crate::sync::VoltaLock;
1213
use log::info;
1314
use semver::Version;
1415

@@ -54,6 +55,8 @@ impl Tool for Npm {
5455
Ok(())
5556
}
5657
fn install(self: Box<Self>, session: &mut Session) -> Fallible<()> {
58+
// Acquire a lock on the Volta directory, if possible, to prevent concurrent changes
59+
let _lock = VoltaLock::acquire();
5760
self.ensure_fetched(session)?;
5861

5962
session

crates/volta-core/src/tool/package/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ impl Tool for Package {
8484
Ok(())
8585
}
8686
fn install(self: Box<Self>, session: &mut Session) -> Fallible<()> {
87+
// Acquire a lock on the Volta directory, if possible, to prevent concurrent changes
88+
let _lock = VoltaLock::acquire();
8789
if self.is_installed() {
8890
info!("Package {} is already installed", self);
8991
Ok(())
@@ -126,7 +128,7 @@ impl Display for Package {
126128
pub fn uninstall(name: &str) -> Fallible<()> {
127129
let home = volta_home()?;
128130
// Acquire a lock on the Volta directory, if possible, to prevent concurrent changes
129-
let _lock = VoltaLock::acquire(home.root()).ok();
131+
let _lock = VoltaLock::acquire();
130132

131133
// if the package config file exists, use that to remove any installed bins and shims
132134
let package_config_file = home.default_package_config_file(name);

crates/volta-core/src/tool/yarn/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use crate::error::{ErrorKind, Fallible};
88
use crate::inventory::yarn_available;
99
use crate::session::Session;
1010
use crate::style::tool_version;
11+
use crate::sync::VoltaLock;
1112
use semver::Version;
1213

1314
mod fetch;
@@ -53,6 +54,8 @@ impl Tool for Yarn {
5354
Ok(())
5455
}
5556
fn install(self: Box<Self>, session: &mut Session) -> Fallible<()> {
57+
// Acquire a lock on the Volta directory, if possible, to prevent concurrent changes
58+
let _lock = VoltaLock::acquire();
5659
self.ensure_fetched(session)?;
5760

5861
session

crates/volta-migrate/src/lib.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,7 @@ impl MigrationState {
134134
pub fn run_migration() -> Fallible<()> {
135135
// Acquire an exclusive lock on the Volta directory, to ensure that no other migrations are running.
136136
// If this fails, however, we still need to run the migration
137-
let home = volta_home()?.root();
138-
match VoltaLock::acquire(home) {
137+
match VoltaLock::acquire() {
139138
Ok(_lock) => {
140139
// The lock was acquired, so we can be confident that no other migrations are running
141140
detect_and_migrate()

0 commit comments

Comments
 (0)