Skip to content

Commit 6919d2d

Browse files
committed
Merge branch 'main' into vivee/test-context
2 parents 707be4c + bc33642 commit 6919d2d

File tree

7 files changed

+266
-48
lines changed

7 files changed

+266
-48
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/icp-cli/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ path = "src/main.rs"
1111

1212
[dependencies]
1313
anyhow.workspace = true
14+
async-trait.workspace = true
1415
bigdecimal.workspace = true
1516
bip32.workspace = true
1617
byte-unit.workspace = true

crates/icp-cli/src/commands/build/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ pub(crate) async fn exec(ctx: &Context, args: &BuildArgs) -> Result<(), CommandE
138138
// Save the wasm artifact
139139
ctx.artifacts
140140
.save(&c.name, &wasm)
141+
.await
141142
.context(CommandError::ArtifactStore)?;
142143

143144
Ok::<_, CommandError>(())

crates/icp-cli/src/commands/canister/install.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ pub(crate) async fn exec(ctx: &Context, args: &InstallArgs) -> Result<(), Comman
159159
})?;
160160

161161
// Lookup the canister build artifact
162-
let wasm = ctx.artifacts.lookup(&c.name)?;
162+
let wasm = ctx.artifacts.lookup(&c.name).await?;
163163

164164
// Retrieve canister status
165165
let (status,) = mgmt.canister_status(&cid).await?;

crates/icp-cli/src/store_artifact.rs

Lines changed: 65 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,33 @@
1-
use std::sync::Mutex;
2-
31
#[cfg(test)]
4-
use std::collections::HashMap;
2+
use std::{collections::HashMap, sync::Mutex};
53

4+
use async_trait::async_trait;
65
use icp::{
7-
fs::{create_dir_all, read, write},
6+
fs::{
7+
lock::{DirectoryStructureLock, PathsAccess},
8+
read, write,
9+
},
810
prelude::*,
911
};
1012
use snafu::{ResultExt, Snafu};
1113

14+
#[async_trait]
1215
/// Trait for accessing and managing canister build artifacts.
1316
pub(crate) trait Access: Sync + Send {
1417
/// Save a canister artifact (WASM) to the store.
15-
fn save(&self, name: &str, wasm: &[u8]) -> Result<(), SaveError>;
18+
async fn save(&self, name: &str, wasm: &[u8]) -> Result<(), SaveError>;
1619

1720
/// Lookup a canister artifact (WASM) from the store.
18-
fn lookup(&self, name: &str) -> Result<Vec<u8>, LookupError>;
21+
async fn lookup(&self, name: &str) -> Result<Vec<u8>, LookupError>;
1922
}
2023

2124
#[derive(Debug, Snafu)]
2225
pub(crate) enum SaveError {
23-
#[snafu(display("failed to create artifacts directory"))]
24-
ArtifactsDir { source: icp::fs::Error },
25-
2626
#[snafu(display("failed to write artifact file"))]
2727
SaveWriteFileError { source: icp::fs::Error },
28+
29+
#[snafu(transparent)]
30+
LockError { source: icp::fs::lock::LockError },
2831
}
2932

3033
#[derive(Debug, Snafu)]
@@ -34,57 +37,71 @@ pub(crate) enum LookupError {
3437

3538
#[snafu(display("could not find artifact for canister '{name}'"))]
3639
LookupArtifactNotFound { name: String },
40+
41+
#[snafu(transparent)]
42+
LockError { source: icp::fs::lock::LockError },
3743
}
3844

3945
pub(crate) struct ArtifactStore {
40-
path: PathBuf,
41-
lock: Mutex<()>,
46+
lock: DirectoryStructureLock<ArtifactPaths>,
47+
}
48+
49+
struct ArtifactPaths {
50+
dir: PathBuf,
51+
}
52+
53+
impl ArtifactPaths {
54+
fn artifact_by_name(&self, name: &str) -> PathBuf {
55+
self.dir.join(name)
56+
}
57+
}
58+
59+
impl PathsAccess for ArtifactPaths {
60+
fn lock_file(&self) -> PathBuf {
61+
self.dir.join(".lock")
62+
}
4263
}
4364

4465
impl ArtifactStore {
4566
pub(crate) fn new(path: &Path) -> Self {
4667
Self {
47-
path: path.to_owned(),
48-
lock: Mutex::new(()),
68+
lock: DirectoryStructureLock::open_or_create(ArtifactPaths {
69+
dir: path.to_owned(),
70+
})
71+
.expect("failed to create artifact store lock"),
4972
}
5073
}
5174
}
5275

76+
#[async_trait]
5377
impl Access for ArtifactStore {
54-
fn save(&self, name: &str, wasm: &[u8]) -> Result<(), SaveError> {
55-
// Lock Artifact Store
56-
let _g = self
57-
.lock
58-
.lock()
59-
.expect("failed to acquire artifact store lock");
60-
61-
// Create artifacts directory
62-
create_dir_all(&self.path).context(ArtifactsDirSnafu)?;
63-
64-
// Store artifact
65-
write(&self.path.join(name), wasm).context(SaveWriteFileSnafu)?;
66-
67-
Ok(())
78+
async fn save(&self, name: &str, wasm: &[u8]) -> Result<(), SaveError> {
79+
self.lock
80+
.with_write(async |store| {
81+
// Save artifact
82+
write(&store.artifact_by_name(name), wasm).context(SaveWriteFileSnafu)?;
83+
Ok(())
84+
})
85+
.await?
6886
}
6987

70-
fn lookup(&self, name: &str) -> Result<Vec<u8>, LookupError> {
71-
// Lock Artifact Store
72-
let _g = self
73-
.lock
74-
.lock()
75-
.expect("failed to acquire artifact store lock");
76-
77-
// Not Found
78-
if !self.path.join(name).exists() {
79-
return Err(LookupError::LookupArtifactNotFound {
80-
name: name.to_owned(),
81-
});
82-
}
83-
84-
// Load artifact
85-
let wasm = read(&self.path.join(name)).context(LookupReadFileSnafu)?;
86-
87-
Ok(wasm)
88+
async fn lookup(&self, name: &str) -> Result<Vec<u8>, LookupError> {
89+
self.lock
90+
.with_read(async |store| {
91+
let artifact = store.artifact_by_name(name);
92+
// Not Found
93+
if !artifact.exists() {
94+
return Err(LookupError::LookupArtifactNotFound {
95+
name: name.to_owned(),
96+
});
97+
}
98+
99+
// Load artifact
100+
let wasm = read(&artifact).context(LookupReadFileSnafu)?;
101+
102+
Ok(wasm)
103+
})
104+
.await?
88105
}
89106
}
90107

@@ -112,14 +129,15 @@ impl Default for MockInMemoryArtifactStore {
112129
}
113130

114131
#[cfg(test)]
132+
#[async_trait]
115133
impl Access for MockInMemoryArtifactStore {
116-
fn save(&self, name: &str, wasm: &[u8]) -> Result<(), SaveError> {
134+
async fn save(&self, name: &str, wasm: &[u8]) -> Result<(), SaveError> {
117135
let mut store = self.store.lock().unwrap();
118136
store.insert(name.to_string(), wasm.to_vec());
119137
Ok(())
120138
}
121139

122-
fn lookup(&self, name: &str) -> Result<Vec<u8>, LookupError> {
140+
async fn lookup(&self, name: &str) -> Result<Vec<u8>, LookupError> {
123141
let store = self.store.lock().unwrap();
124142

125143
match store.get(name) {

crates/icp/src/fs/lock.rs

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
//! File locking abstractions to make directory locks easy and safe.
2+
//!
3+
//! Directory structures are typically represented by a struct containing the directory path,
4+
//! which then has methods for getting files or directories within it. This struct should implement
5+
//! `PathsAccess`, and then instead of passing it around directly, it should be stored in a
6+
//! [`DirectoryStructureLock`]. This ensures that paths cannot be accessed without holding the appropriate
7+
//! lock.
8+
//!
9+
//! Temporary locks can be acquired using the `with_read` and `with_write` methods, which take
10+
//! async closures. For locks that should be stored in a structure, `into_read` and `into_write` can be used
11+
//! to convert the lock into an owned guard.
12+
//!
13+
//! When making low-level functions that might be composed into higher-level operations, these functions should
14+
//! typically take `LRead<&T>` or `LWrite<&T>` parameters, rather than `&T`. This makes sure the composed function
15+
//! will demand the right kind of lock, when writes are hidden in what looks at first glance like a read operation.
16+
17+
use crate::{fs, prelude::*};
18+
use snafu::{ResultExt, Snafu};
19+
use std::{fs::File, io, ops::Deref};
20+
use tokio::{sync::RwLock, task::spawn_blocking};
21+
22+
/// Directory lock ensuring safe concurrency around filesystem operations.
23+
pub struct DirectoryStructureLock<T: PathsAccess> {
24+
paths_access: T,
25+
lock_file: RwLock<File>,
26+
lock_path: PathBuf,
27+
}
28+
29+
/// A directory structure, typically with methods to access specific paths within it.
30+
///
31+
/// One file within it is selected as the file for advisory locks.
32+
pub trait PathsAccess: Send + Sync + 'static {
33+
/// Path to the canonical file for locking the directory structure. Usually `$dir/.lock`.
34+
fn lock_file(&self) -> PathBuf;
35+
}
36+
37+
impl<T: PathsAccess> DirectoryStructureLock<T> {
38+
/// Creates a new lock, implicitly calling [`fs::create_dir_all`] on the parent.
39+
pub fn open_or_create(paths_access: T) -> Result<Self, LockError> {
40+
let lock_path = paths_access.lock_file();
41+
fs::create_dir_all(lock_path.parent().unwrap())?;
42+
let lock_file =
43+
File::create(&lock_path).context(OpenLockFileFailedSnafu { path: &lock_path })?;
44+
Ok(Self {
45+
paths_access,
46+
lock_file: RwLock::const_new(lock_file),
47+
lock_path,
48+
})
49+
}
50+
51+
/// Converts the lock structure into an owned read-lock.
52+
pub async fn into_read(self) -> Result<DirectoryStructureGuardOwned<LRead<T>>, LockError> {
53+
spawn_blocking(move || {
54+
let lock_file = self.lock_file.into_inner();
55+
lock_file.lock_shared().context(LockFailedSnafu {
56+
lock_path: self.lock_path,
57+
})?;
58+
Ok(DirectoryStructureGuardOwned {
59+
paths_access: LRead(self.paths_access),
60+
guard: lock_file,
61+
})
62+
})
63+
.await
64+
.unwrap()
65+
}
66+
67+
/// Converts the lock structure into an owned write-lock.
68+
pub async fn into_write(self) -> Result<DirectoryStructureGuardOwned<LWrite<T>>, LockError> {
69+
spawn_blocking(move || {
70+
let lock_file = self.lock_file.into_inner();
71+
lock_file.lock().context(LockFailedSnafu {
72+
lock_path: self.lock_path,
73+
})?;
74+
Ok(DirectoryStructureGuardOwned {
75+
paths_access: LWrite(self.paths_access),
76+
guard: lock_file,
77+
})
78+
})
79+
.await
80+
.unwrap()
81+
}
82+
83+
/// Accesses the directory structure under a read lock.
84+
pub async fn with_read<R>(&self, f: impl AsyncFnOnce(LRead<&T>) -> R) -> Result<R, LockError> {
85+
let guard = self.lock_file.read().await;
86+
let lock_file = guard.try_clone().context(HandleCloneFailedSnafu {
87+
path: &self.lock_path,
88+
})?;
89+
spawn_blocking(move || lock_file.lock_shared())
90+
.await
91+
.unwrap()
92+
.context(LockFailedSnafu {
93+
lock_path: &self.lock_path,
94+
})?;
95+
let ret = f(LRead(&self.paths_access)).await;
96+
guard.unlock().context(LockFailedSnafu {
97+
lock_path: &self.lock_path,
98+
})?;
99+
Ok(ret)
100+
}
101+
102+
/// Accesses the directory structure under a write lock.
103+
pub async fn with_write<R>(
104+
&self,
105+
f: impl AsyncFnOnce(LWrite<&T>) -> R,
106+
) -> Result<R, LockError> {
107+
let guard = self.lock_file.write().await;
108+
let lock_file = guard.try_clone().context(HandleCloneFailedSnafu {
109+
path: &self.lock_path,
110+
})?;
111+
spawn_blocking(move || lock_file.lock())
112+
.await
113+
.unwrap()
114+
.context(LockFailedSnafu {
115+
lock_path: &self.lock_path,
116+
})?;
117+
let ret = f(LWrite(&self.paths_access)).await;
118+
guard.unlock().context(LockFailedSnafu {
119+
lock_path: &self.lock_path,
120+
})?;
121+
Ok(ret)
122+
}
123+
}
124+
125+
#[derive(Debug, Snafu)]
126+
pub enum LockError {
127+
#[snafu(transparent)]
128+
CreateDirFailed { source: crate::fs::Error },
129+
#[snafu(display("failed to create or open lock file '{path}'"))]
130+
OpenLockFileFailed { source: io::Error, path: PathBuf },
131+
#[snafu(display("failed to lock the file '{lock_path}'"))]
132+
LockFailed {
133+
source: io::Error,
134+
lock_path: PathBuf,
135+
},
136+
#[snafu(display("failed to clone lock file handle '{path}'"))]
137+
HandleCloneFailed { source: io::Error, path: PathBuf },
138+
}
139+
140+
/// File lock guard. Do not use as a temporary in an expression - if you are making a temporary lock, use `with_*`.
141+
#[clippy::has_significant_drop]
142+
pub struct DirectoryStructureGuardOwned<T> {
143+
paths_access: T,
144+
guard: File,
145+
}
146+
147+
impl<T> Deref for DirectoryStructureGuardOwned<T> {
148+
type Target = T;
149+
150+
fn deref(&self) -> &Self::Target {
151+
&self.paths_access
152+
}
153+
}
154+
155+
impl<T> Drop for DirectoryStructureGuardOwned<T> {
156+
fn drop(&mut self) {
157+
_ = self.guard.unlock();
158+
}
159+
}
160+
161+
pub struct LRead<T>(T);
162+
pub struct LWrite<T>(T);
163+
164+
impl<T> Deref for LRead<T> {
165+
type Target = T;
166+
167+
fn deref(&self) -> &Self::Target {
168+
&self.0
169+
}
170+
}
171+
172+
impl<T> Deref for LWrite<T> {
173+
type Target = T;
174+
175+
fn deref(&self) -> &Self::Target {
176+
&self.0
177+
}
178+
}
179+
180+
impl<'a, T> LWrite<&'a T> {
181+
pub fn read(&self) -> LRead<&'a T> {
182+
LRead(self.0)
183+
}
184+
}
185+
186+
impl<T> LWrite<T> {
187+
pub fn as_ref(&self) -> LWrite<&T> {
188+
LWrite(&self.0)
189+
}
190+
}
191+
192+
impl<T> LRead<T> {
193+
pub fn as_ref(&self) -> LRead<&T> {
194+
LRead(&self.0)
195+
}
196+
}

0 commit comments

Comments
 (0)