Skip to content

Commit a45d21e

Browse files
committed
refactor(hm-util): unify blocking wrappers via block_in_place
Remove duplicate sync code paths — blocking:: now shells out to the async API through tokio::task::block_in_place. Platform-specific rename logic moves from standalone _sync functions to private _impl helpers positioned after the public async fn.
1 parent 716a455 commit a45d21e

2 files changed

Lines changed: 62 additions & 74 deletions

File tree

crates/hm-util/src/os/fs.rs

Lines changed: 60 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -39,21 +39,13 @@ fn write_atomic_restricted_sync(
3939

4040
write_file_with_mode_sync(&tmp_path, contents, file_mode)?;
4141

42-
let persist_result = atomic_rename_over_sync(&tmp_path, path);
42+
let persist_result = atomic_rename_over_impl(&tmp_path, path);
4343
if persist_result.is_err() {
4444
let _ = std::fs::remove_file(&tmp_path);
4545
}
4646
persist_result
4747
}
4848

49-
fn remove_if_exists_sync(path: &Path) -> io::Result<()> {
50-
match std::fs::remove_file(path) {
51-
Ok(()) => Ok(()),
52-
Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(()),
53-
Err(e) => Err(e),
54-
}
55-
}
56-
5749
#[cfg(unix)]
5850
fn create_dir_with_mode_sync(dir: &Path, mode: u32) -> io::Result<()> {
5951
use std::os::unix::fs::{DirBuilderExt, PermissionsExt};
@@ -96,56 +88,6 @@ fn write_file_with_mode_sync(path: &Path, contents: &[u8], _mode: u32) -> io::Re
9688
std::fs::write(path, contents)
9789
}
9890

99-
// ---------------------------------------------------------------------------
100-
// Cross-platform atomic rename
101-
// ---------------------------------------------------------------------------
102-
103-
#[cfg(unix)]
104-
fn atomic_rename_over_sync(from: &Path, to: &Path) -> io::Result<()> {
105-
std::fs::rename(from, to)
106-
}
107-
108-
#[cfg(windows)]
109-
fn atomic_rename_over_sync(from: &Path, to: &Path) -> io::Result<()> {
110-
use windows::core::HSTRING;
111-
use windows::Win32::Storage::FileSystem::{
112-
MoveFileExW, ReplaceFileW,
113-
MOVEFILE_REPLACE_EXISTING, MOVEFILE_WRITE_THROUGH,
114-
REPLACEFILE_IGNORE_MERGE_ERRORS,
115-
};
116-
117-
let from_w = HSTRING::from(from.as_os_str());
118-
let to_w = HSTRING::from(to.as_os_str());
119-
120-
// ReplaceFileW preserves ACLs and alternate data streams on the
121-
// target, but requires the target to already exist.
122-
if to.exists() {
123-
let result = unsafe {
124-
ReplaceFileW(
125-
&to_w,
126-
&from_w,
127-
windows::core::PCWSTR::null(),
128-
REPLACEFILE_IGNORE_MERGE_ERRORS,
129-
None,
130-
None,
131-
)
132-
};
133-
return result.map_err(|e| io::Error::new(io::ErrorKind::Other, e));
134-
}
135-
136-
// Target doesn't exist yet — fall back to MoveFileExW which handles
137-
// both cases but doesn't preserve target metadata (irrelevant here
138-
// since there is no target).
139-
let result = unsafe {
140-
MoveFileExW(
141-
&from_w,
142-
&to_w,
143-
MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH,
144-
)
145-
};
146-
result.map_err(|e| io::Error::new(io::ErrorKind::Other, e))
147-
}
148-
14991
/// Write `contents` to `path` atomically with `file_mode`, ensuring the
15092
/// parent directory exists and is set to `dir_mode`.
15193
///
@@ -190,11 +132,52 @@ pub async fn atomic_rename_over(
190132
) -> io::Result<()> {
191133
let from = from.as_ref().to_owned();
192134
let to = to.as_ref().to_owned();
193-
tokio::task::spawn_blocking(move || atomic_rename_over_sync(&from, &to))
135+
tokio::task::spawn_blocking(move || atomic_rename_over_impl(&from, &to))
194136
.await
195137
.map_err(io::Error::other)?
196138
}
197139

140+
#[cfg(unix)]
141+
fn atomic_rename_over_impl(from: &Path, to: &Path) -> io::Result<()> {
142+
std::fs::rename(from, to)
143+
}
144+
145+
#[cfg(windows)]
146+
fn atomic_rename_over_impl(from: &Path, to: &Path) -> io::Result<()> {
147+
use windows::core::HSTRING;
148+
use windows::Win32::Storage::FileSystem::{
149+
MoveFileExW, ReplaceFileW,
150+
MOVEFILE_REPLACE_EXISTING, MOVEFILE_WRITE_THROUGH,
151+
REPLACEFILE_IGNORE_MERGE_ERRORS,
152+
};
153+
154+
let from_w = HSTRING::from(from.as_os_str());
155+
let to_w = HSTRING::from(to.as_os_str());
156+
157+
if to.exists() {
158+
let result = unsafe {
159+
ReplaceFileW(
160+
&to_w,
161+
&from_w,
162+
windows::core::PCWSTR::null(),
163+
REPLACEFILE_IGNORE_MERGE_ERRORS,
164+
None,
165+
None,
166+
)
167+
};
168+
return result.map_err(|e| io::Error::new(io::ErrorKind::Other, e));
169+
}
170+
171+
let result = unsafe {
172+
MoveFileExW(
173+
&from_w,
174+
&to_w,
175+
MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH,
176+
)
177+
};
178+
result.map_err(|e| io::Error::new(io::ErrorKind::Other, e))
179+
}
180+
198181
/// Remove a file if it exists; silently return `Ok(())` if it does not.
199182
///
200183
/// This is the async counterpart of [`blocking::remove_if_exists`];
@@ -212,12 +195,17 @@ pub async fn remove_file_if_exists(path: impl AsRef<Path>) -> io::Result<()> {
212195
}
213196
}
214197

215-
/// Synchronous (blocking) wrappers for callers that cannot use async,
216-
/// such as extism `host_fn` callbacks.
198+
/// Synchronous (blocking) wrappers that shell out to the async API
199+
/// via `tokio::task::block_in_place`. Safe to call from sync contexts
200+
/// that run inside a tokio runtime (e.g. extism `host_fn` callbacks).
217201
pub mod blocking {
218202
use std::io;
219203
use std::path::Path;
220204

205+
fn block_on<F: std::future::Future<Output = io::Result<()>>>(f: F) -> io::Result<()> {
206+
tokio::task::block_in_place(|| tokio::runtime::Handle::current().block_on(f))
207+
}
208+
221209
/// Write `contents` to `path` atomically with `file_mode`, ensuring the
222210
/// parent directory exists and is set to `dir_mode`.
223211
///
@@ -235,7 +223,7 @@ pub mod blocking {
235223
file_mode: u32,
236224
dir_mode: u32,
237225
) -> io::Result<()> {
238-
super::write_atomic_restricted_sync(path.as_ref(), contents.as_ref(), file_mode, dir_mode)
226+
block_on(super::write_atomic_restricted(path, contents, file_mode, dir_mode))
239227
}
240228

241229
/// Remove a file if it exists; silently return `Ok(())` if it does not.
@@ -245,7 +233,7 @@ pub mod blocking {
245233
/// Returns an error if `remove_file` fails for any reason other than
246234
/// `NotFound`.
247235
pub fn remove_if_exists(path: impl AsRef<Path>) -> io::Result<()> {
248-
super::remove_if_exists_sync(path.as_ref())
236+
block_on(super::remove_file_if_exists(path))
249237
}
250238
}
251239

@@ -255,8 +243,8 @@ mod tests {
255243
use super::blocking;
256244
use std::os::unix::fs::PermissionsExt;
257245

258-
#[test]
259-
fn writes_file_and_dir_with_requested_modes() {
246+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
247+
async fn writes_file_and_dir_with_requested_modes() {
260248
let tmp = tempfile::tempdir().unwrap();
261249
let target = tmp.path().join("sub").join("creds");
262250
blocking::write_atomic_restricted(&target, b"hello", 0o600, 0o700).unwrap();
@@ -275,8 +263,8 @@ mod tests {
275263
assert_eq!(dir_mode, 0o700, "dir mode must be 0o700, got {dir_mode:o}");
276264
}
277265

278-
#[test]
279-
fn overwrites_existing_file_preserving_mode() {
266+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
267+
async fn overwrites_existing_file_preserving_mode() {
280268
let tmp = tempfile::tempdir().unwrap();
281269
let target = tmp.path().join("creds");
282270
blocking::write_atomic_restricted(&target, b"v1", 0o600, 0o700).unwrap();
@@ -287,8 +275,8 @@ mod tests {
287275
assert_eq!(mode, 0o600);
288276
}
289277

290-
#[test]
291-
fn tightens_existing_dir_with_looser_mode() {
278+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
279+
async fn tightens_existing_dir_with_looser_mode() {
292280
let tmp = tempfile::tempdir().unwrap();
293281
let dir = tmp.path().join("loose");
294282
std::fs::create_dir(&dir).unwrap();
@@ -301,8 +289,8 @@ mod tests {
301289
assert_eq!(dir_mode, 0o700);
302290
}
303291

304-
#[test]
305-
fn remove_if_exists_is_idempotent() {
292+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
293+
async fn remove_if_exists_is_idempotent() {
306294
let tmp = tempfile::tempdir().unwrap();
307295
let target = tmp.path().join("nothing");
308296
blocking::remove_if_exists(&target).unwrap();

crates/hm/src/creds_store.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,8 @@ mod tests {
9292
}
9393
}
9494

95-
#[test]
96-
fn round_trip() {
95+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
96+
async fn round_trip() {
9797
with_home(|| {
9898
assert_eq!(get("svc", "acct"), None);
9999
set("svc", "acct", "shh");

0 commit comments

Comments
 (0)