mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(reset): release log file handle + queue locked entries on Windows reboot (#2668)
This commit is contained in:
+183
-18
@@ -36,6 +36,8 @@ mod native_notifications;
|
||||
mod notification_settings;
|
||||
mod process_kill;
|
||||
mod process_recovery;
|
||||
#[cfg(target_os = "windows")]
|
||||
mod reset_reboot_schedule;
|
||||
mod screen_capture;
|
||||
mod slack_scanner;
|
||||
mod telegram_scanner;
|
||||
@@ -384,6 +386,18 @@ async fn reset_local_data(
|
||||
state.inner().shutdown().await;
|
||||
log::info!("[core] reset_local_data: embedded core stopped");
|
||||
|
||||
// ── 3b. Release the host-process log file handle (issue #1615) ──────
|
||||
//
|
||||
// The daily-rotating log appender at `<data_dir>/logs/openhuman-*.log`
|
||||
// is owned by *this* Tauri host process, not by the embedded core
|
||||
// tokio task — so `shutdown()` above does not release it. On Windows
|
||||
// that lingering OS file handle causes `remove_dir_all(.openhuman)`
|
||||
// below to fail with `ERROR_SHARING_VIOLATION` (os error 32). Drop
|
||||
// the writer guard now so the background flushing thread exits and
|
||||
// the file handle is closed before the removal walks the tree.
|
||||
let log_guard_dropped = openhuman_core::core::logging::shutdown_file_guard();
|
||||
log::info!("[core] reset_local_data: shutdown_file_guard dropped guard = {log_guard_dropped}");
|
||||
|
||||
// ── 4. Remove the paths ─────────────────────────────────────────────
|
||||
//
|
||||
// Missing entries are non-fatal: the user may already have manually
|
||||
@@ -450,23 +464,125 @@ fn is_windows_file_lock_error(error: &std::io::Error) -> bool {
|
||||
cfg!(windows) && is_windows_file_lock_raw_os_error(error.raw_os_error())
|
||||
}
|
||||
|
||||
/// Returns:
|
||||
/// * `Ok(())` — the underlying remove failure should be swallowed (e.g.
|
||||
/// the path disappeared between the failed `remove_*` call and the
|
||||
/// reboot-fallback walk, so there is nothing left to clean up).
|
||||
/// * `Err(msg)` — a user-facing failure message the caller should surface
|
||||
/// to the UI / propagate up the reset flow.
|
||||
fn reset_local_data_delete_error(
|
||||
label: &str,
|
||||
path: &std::path::Path,
|
||||
error: &std::io::Error,
|
||||
) -> String {
|
||||
) -> Result<(), String> {
|
||||
if is_windows_file_lock_error(error) {
|
||||
log::warn!(
|
||||
"[core] reset_local_data: Windows file lock blocked removal of {label} at {}: {error}",
|
||||
path.display()
|
||||
);
|
||||
return format!(
|
||||
"Failed to remove {label} at {} because it is locked by another OpenHuman window or process. Close all OpenHuman windows and try again. ({error})",
|
||||
path.display()
|
||||
);
|
||||
|
||||
// Fallback: queue the still-locked sub-tree for deletion on the
|
||||
// next Windows boot via MoveFileExW + MOVEFILE_DELAY_UNTIL_REBOOT.
|
||||
// By this point in `reset_local_data` we have already:
|
||||
// * shut down the embedded core (drops every SQLite/log handle
|
||||
// the core task held), and
|
||||
// * released the host-process log appender via
|
||||
// `shutdown_file_guard()` (drops the rolling log file handle).
|
||||
// So any remaining lock now comes from *outside* this process —
|
||||
// anti-virus / file indexer / sibling app / Explorer — and cannot
|
||||
// be released by closing more OpenHuman windows. See issue #1615.
|
||||
#[cfg(target_os = "windows")]
|
||||
{
|
||||
return schedule_reboot_delete_or_describe(label, path, error);
|
||||
}
|
||||
// `is_windows_file_lock_error` is gated on `cfg!(windows)`, so on
|
||||
// Linux/macOS this branch is unreachable at runtime — but cargo
|
||||
// still type-checks the file for those targets and needs a value
|
||||
// of type `String`.
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
{
|
||||
return Err(format!(
|
||||
"Failed to remove {label} at {} because it is locked by another OpenHuman window or process. Close all OpenHuman windows and try again. ({error})",
|
||||
path.display()
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
format!("Failed to remove {label} at {}: {error}", path.display())
|
||||
Err(format!(
|
||||
"Failed to remove {label} at {}: {error}",
|
||||
path.display()
|
||||
))
|
||||
}
|
||||
|
||||
/// Windows-only: ask the session manager to delete `path` (and its
|
||||
/// children if it is a directory) on the next reboot, and return either a
|
||||
/// user-facing message describing the outcome or `Ok(())` when the
|
||||
/// underlying failure should be treated as already-cleaned-up.
|
||||
#[cfg(target_os = "windows")]
|
||||
fn schedule_reboot_delete_or_describe(
|
||||
label: &str,
|
||||
path: &std::path::Path,
|
||||
original_error: &std::io::Error,
|
||||
) -> Result<(), String> {
|
||||
match reset_reboot_schedule::schedule_path_for_reboot_deletion(path) {
|
||||
Ok(summary) => {
|
||||
log::info!(
|
||||
"[core] reset_local_data: scheduled {label} at {} for reboot deletion (files={}, dirs={})",
|
||||
path.display(),
|
||||
summary.files,
|
||||
summary.dirs
|
||||
);
|
||||
Err(format!(
|
||||
"Couldn't remove {label} at {} right now because another process is holding it open ({original_error}). {} files and {} folders have been queued for deletion the next time you restart Windows — restart soon to finish the reset.",
|
||||
path.display(),
|
||||
summary.files,
|
||||
summary.dirs,
|
||||
))
|
||||
}
|
||||
// Race condition: the still-locked path disappeared between the
|
||||
// `remove_*` call that failed with `ERROR_SHARING_VIOLATION` and
|
||||
// the metadata read inside the reboot-schedule walk. Whoever else
|
||||
// held the handle has already finished cleaning up, so the reset
|
||||
// goal is achieved — swallow the original lock error and treat
|
||||
// this as success. The empty partial schedule (no entries queued
|
||||
// yet) is what distinguishes "vanished cleanly" from "started
|
||||
// walking, then hit a real error."
|
||||
Err(failure)
|
||||
if failure.error.kind() == std::io::ErrorKind::NotFound
|
||||
&& failure.partial.total() == 0 =>
|
||||
{
|
||||
log::info!(
|
||||
"[core] reset_local_data: {label} at {} disappeared between lock failure and reboot fallback; treating as removed",
|
||||
path.display(),
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
Err(failure) => {
|
||||
let partial_total = failure.partial.total();
|
||||
log::error!(
|
||||
"[core] reset_local_data: reboot delete fallback failed for {label} at {}: {} (partial schedule: files={}, dirs={})",
|
||||
path.display(),
|
||||
failure.error,
|
||||
failure.partial.files,
|
||||
failure.partial.dirs,
|
||||
);
|
||||
if partial_total == 0 {
|
||||
Err(format!(
|
||||
"Failed to remove {label} at {} because it is locked by another OpenHuman window or process, and scheduling deletion on next reboot also failed ({}). Close all OpenHuman windows and try again. ({original_error})",
|
||||
path.display(),
|
||||
failure.error,
|
||||
))
|
||||
} else {
|
||||
Err(format!(
|
||||
"Failed to remove {label} at {} because it is locked by another OpenHuman window or process. {} files and {} folders were queued for the next reboot before scheduling failed ({}); the rest still needs manual cleanup. Close all OpenHuman windows and try again. ({original_error})",
|
||||
path.display(),
|
||||
failure.partial.files,
|
||||
failure.partial.dirs,
|
||||
failure.error,
|
||||
))
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Call the core's `config_get_data_paths` RPC and parse the response.
|
||||
@@ -536,7 +652,7 @@ async fn remove_path_if_exists(path: &std::path::Path, label: &str) -> Result<()
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
Err(e) => Err(reset_local_data_delete_error(label, path, &e)),
|
||||
Err(e) => reset_local_data_delete_error(label, path, &e),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -557,7 +673,7 @@ async fn remove_dir_if_exists(path: &std::path::Path, label: &str) -> Result<(),
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
Err(e) => Err(reset_local_data_delete_error(label, path, &e)),
|
||||
Err(e) => reset_local_data_delete_error(label, path, &e),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3620,28 +3736,77 @@ mod tests {
|
||||
#[test]
|
||||
fn reset_local_data_delete_error_keeps_generic_message_for_other_errors() {
|
||||
let err = std::io::Error::from(std::io::ErrorKind::PermissionDenied);
|
||||
let msg = reset_local_data_delete_error(
|
||||
let result = reset_local_data_delete_error(
|
||||
"current openhuman dir",
|
||||
std::path::Path::new("/tmp/openhuman"),
|
||||
&err,
|
||||
);
|
||||
|
||||
let msg = result.expect_err("non-lock errors must still surface to the UI");
|
||||
assert!(msg.starts_with("Failed to remove current openhuman dir at /tmp/openhuman:"));
|
||||
assert!(!msg.contains("Close all OpenHuman windows and try again"));
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn reset_local_data_delete_error_explains_windows_file_locks() {
|
||||
let err = std::io::Error::from_raw_os_error(32);
|
||||
let msg = reset_local_data_delete_error(
|
||||
"current openhuman dir",
|
||||
std::path::Path::new("C:\\Users\\me\\.openhuman"),
|
||||
&err,
|
||||
);
|
||||
fn reset_local_data_delete_error_swallows_lock_failure_when_path_disappeared() {
|
||||
// Race condition the reboot fallback now handles: the locked path
|
||||
// was gone by the time `schedule_path_for_reboot_deletion` ran its
|
||||
// `symlink_metadata` probe, so the reset goal is already met. The
|
||||
// helper must return `Ok(())` rather than surfacing a confusing
|
||||
// "couldn't remove (it's not there)" toast.
|
||||
let dir = tempfile::tempdir().expect("tempdir for reset error test");
|
||||
let missing = dir.path().join("definitely-not-there");
|
||||
|
||||
assert!(msg.contains("locked by another OpenHuman window or process"));
|
||||
assert!(msg.contains("Close all OpenHuman windows and try again"));
|
||||
let err = std::io::Error::from_raw_os_error(32);
|
||||
let result = reset_local_data_delete_error("current openhuman dir", &missing, &err);
|
||||
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"expected NotFound + empty partial schedule to be swallowed as success, got {result:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn reset_local_data_delete_error_reports_reboot_schedule_counts() {
|
||||
// When the lock fallback can walk a real directory tree, the user
|
||||
// message should report how much has been queued so the support
|
||||
// log preserves "what was actually scheduled". Scheduling itself
|
||||
// may still fail at the MoveFileExW step in unprivileged test
|
||||
// processes (the registry key write requires administrator); the
|
||||
// fallback then carries a partial schedule that the error path
|
||||
// surfaces, so both branches must keep mentioning the lock cause
|
||||
// *and* expose either the queued counts or the schedule failure.
|
||||
let dir = tempfile::tempdir().expect("tempdir for reset error test");
|
||||
let target = dir.path().join("reset-mock");
|
||||
std::fs::create_dir_all(target.join("nested")).expect("mkdir nested");
|
||||
std::fs::write(target.join("a.txt"), b"x").expect("write a.txt");
|
||||
std::fs::write(target.join("nested").join("b.txt"), b"y").expect("write b.txt");
|
||||
|
||||
let err = std::io::Error::from_raw_os_error(32);
|
||||
let result = reset_local_data_delete_error("current openhuman dir", &target, &err);
|
||||
|
||||
// Path exists on disk, so the fallback must surface the outcome —
|
||||
// either an "all-queued" success-but-needs-reboot message (admin)
|
||||
// or one of the failure flavours (non-admin).
|
||||
let msg = result
|
||||
.expect_err("path exists, fallback must report queued counts or scheduling failure");
|
||||
let admin_path = msg.contains("queued for deletion the next time you restart Windows")
|
||||
&& msg.contains("2 files and 2 folders");
|
||||
let user_full_fail = msg.contains("scheduling deletion on next reboot also failed");
|
||||
let user_partial = msg.contains("queued for the next reboot before scheduling failed");
|
||||
assert!(
|
||||
admin_path || user_full_fail || user_partial,
|
||||
"expected reboot-scheduled, fully-failed, or partial-fail message, got: {msg}"
|
||||
);
|
||||
// Whatever branch we land on, the user must still be told the lock
|
||||
// is what blocked the immediate removal.
|
||||
assert!(
|
||||
msg.contains("locked by another OpenHuman window or process")
|
||||
|| msg.contains("another process is holding it open"),
|
||||
"missing lock cause: {msg}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Tests for setup_tray conditional compilation
|
||||
|
||||
@@ -0,0 +1,358 @@
|
||||
//! Windows-only fallback for `reset_local_data` (issue #1615).
|
||||
//!
|
||||
//! When the in-process `remove_dir_all` step fails because a third-party
|
||||
//! process (anti-virus, file-indexer, sibling OpenHuman window) still holds
|
||||
//! an open handle inside the `.openhuman` tree, Windows returns
|
||||
//! `ERROR_SHARING_VIOLATION` (os error 32) / `ERROR_LOCK_VIOLATION` (33)
|
||||
//! and the user is stuck — see PR #2395 / #1811, which surface a "close all
|
||||
//! OpenHuman windows" prompt but cannot break a foreign lock.
|
||||
//!
|
||||
//! This module walks the still-present sub-tree depth-first and asks the
|
||||
//! Windows Session Manager to delete each entry at next boot via
|
||||
//! `MoveFileExW(src, NULL, MOVEFILE_DELAY_UNTIL_REBOOT)`. The session
|
||||
//! manager requires that directories be empty when boot-time deletion
|
||||
//! runs, so children are scheduled before their parent.
|
||||
//!
|
||||
//! Reference:
|
||||
//! https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-movefileexw
|
||||
//!
|
||||
//! Privileges: `MoveFileExW(.., NULL, MOVEFILE_DELAY_UNTIL_REBOOT)` writes
|
||||
//! to `HKLM\SYSTEM\CurrentControlSet\Control\Session Manager\PendingFileRenameOperations`
|
||||
//! (the boot-time session manager reads from HKLM, not the per-user hive),
|
||||
//! so the call **may fail for non-administrator users** with `ERROR_ACCESS_DENIED`.
|
||||
//! That is by design — Microsoft documents the elevation requirement on the
|
||||
//! `MOVEFILE_DELAY_UNTIL_REBOOT` flag — and the caller in `lib.rs` handles
|
||||
//! the failure path gracefully: it preserves the original lock error plus
|
||||
//! the schedule failure reason and falls back to the "close all OpenHuman
|
||||
//! windows and try again" guidance from PR #2395 / #1811.
|
||||
|
||||
#![cfg(target_os = "windows")]
|
||||
|
||||
use std::io;
|
||||
use std::os::windows::ffi::OsStrExt;
|
||||
use std::path::Path;
|
||||
|
||||
use windows_sys::Win32::Storage::FileSystem::{MoveFileExW, MOVEFILE_DELAY_UNTIL_REBOOT};
|
||||
|
||||
/// Tally of entries handed off to `MoveFileExW`, returned to the caller so
|
||||
/// it can log and surface (e.g. "scheduled 142 files / 14 dirs for deletion
|
||||
/// on next reboot") instead of just an opaque "ok".
|
||||
///
|
||||
/// `partial` is `true` when the walk aborted mid-tree (e.g. a directory
|
||||
/// became unreadable, or an individual `MoveFileExW` call failed). In that
|
||||
/// case `files` / `dirs` represent **only** what was queued before the
|
||||
/// failure point — useful for support logs to distinguish "everything is
|
||||
/// queued" from "some of the tree is queued but the rest still needs
|
||||
/// manual cleanup." Pair with the `Result::Err` returned by
|
||||
/// [`schedule_path_for_reboot_deletion`] for the cause.
|
||||
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct RebootDeletionSchedule {
|
||||
pub files: u32,
|
||||
pub dirs: u32,
|
||||
pub partial: bool,
|
||||
}
|
||||
|
||||
impl RebootDeletionSchedule {
|
||||
pub fn total(&self) -> u32 {
|
||||
self.files.saturating_add(self.dirs)
|
||||
}
|
||||
}
|
||||
|
||||
/// Schedule `path` (and everything under it if it is a directory) for
|
||||
/// deletion on the next reboot via `MoveFileExW(_, NULL, MOVEFILE_DELAY_UNTIL_REBOOT)`.
|
||||
///
|
||||
/// Strategy:
|
||||
/// * Regular files / symlinks → scheduled directly.
|
||||
/// * Directories → children scheduled first (depth-first), then the
|
||||
/// directory itself once its contents are queued.
|
||||
///
|
||||
/// `path` not existing on disk yields `Err(RebootDeletionFailure { error: NotFound, .. })` —
|
||||
/// callers can choose to treat that as a no-op since "nothing to remove" is
|
||||
/// the same outcome.
|
||||
///
|
||||
/// On error the failure carries a partially-populated `RebootDeletionSchedule`
|
||||
/// (`partial = true`) so the caller can surface "we queued N files and M
|
||||
/// folders before scheduling failed" instead of just the bare io error.
|
||||
/// The walk is depth-first, so the counts reflect entries queued *before*
|
||||
/// the failing step.
|
||||
pub fn schedule_path_for_reboot_deletion(
|
||||
path: &Path,
|
||||
) -> Result<RebootDeletionSchedule, RebootDeletionFailure> {
|
||||
schedule_path_with_scheduler(path, &mut schedule_one)
|
||||
}
|
||||
|
||||
/// Internal seam used by both [`schedule_path_for_reboot_deletion`] (which
|
||||
/// passes the real `MoveFileExW` step as `scheduler`) and the unit tests
|
||||
/// (which pass an injectable `Ok(())` stub so the traversal/counting logic
|
||||
/// can be exercised on every dev machine without needing administrator
|
||||
/// rights or actually queuing reboot-time deletions).
|
||||
fn schedule_path_with_scheduler<F>(
|
||||
path: &Path,
|
||||
scheduler: &mut F,
|
||||
) -> Result<RebootDeletionSchedule, RebootDeletionFailure>
|
||||
where
|
||||
F: FnMut(&Path) -> io::Result<()>,
|
||||
{
|
||||
let metadata = std::fs::symlink_metadata(path).map_err(|error| RebootDeletionFailure {
|
||||
error,
|
||||
partial: RebootDeletionSchedule {
|
||||
partial: true,
|
||||
..RebootDeletionSchedule::default()
|
||||
},
|
||||
})?;
|
||||
let mut summary = RebootDeletionSchedule::default();
|
||||
match schedule_inner(path, &metadata, &mut summary, scheduler) {
|
||||
Ok(()) => Ok(summary),
|
||||
Err(error) => {
|
||||
summary.partial = true;
|
||||
Err(RebootDeletionFailure {
|
||||
error,
|
||||
partial: summary,
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Pair of `(io::Error, partial schedule)` returned when the depth-first
|
||||
/// walk aborts mid-tree. The `partial` field records what was queued via
|
||||
/// `MoveFileExW` *before* the failure point so the caller can include the
|
||||
/// counts in user-facing copy and support logs ("123 files / 7 folders
|
||||
/// were queued for the next reboot before scheduling failed: <reason>").
|
||||
#[derive(Debug)]
|
||||
pub struct RebootDeletionFailure {
|
||||
pub error: io::Error,
|
||||
pub partial: RebootDeletionSchedule,
|
||||
}
|
||||
|
||||
impl std::fmt::Display for RebootDeletionFailure {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
write!(f, "{}", self.error)
|
||||
}
|
||||
}
|
||||
|
||||
impl std::error::Error for RebootDeletionFailure {
|
||||
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
|
||||
Some(&self.error)
|
||||
}
|
||||
}
|
||||
|
||||
fn schedule_inner<F>(
|
||||
path: &Path,
|
||||
metadata: &std::fs::Metadata,
|
||||
summary: &mut RebootDeletionSchedule,
|
||||
scheduler: &mut F,
|
||||
) -> io::Result<()>
|
||||
where
|
||||
F: FnMut(&Path) -> io::Result<()>,
|
||||
{
|
||||
// Symlinked directories must NOT be descended into — the lock lives
|
||||
// on the link target, not the link itself, and following would queue
|
||||
// unrelated paths for deletion. Treat symlinks (file or dir) as a
|
||||
// single leaf entry.
|
||||
if metadata.is_dir() && !metadata.file_type().is_symlink() {
|
||||
for entry in std::fs::read_dir(path)? {
|
||||
let entry = entry?;
|
||||
let child_meta = entry.metadata()?;
|
||||
schedule_inner(&entry.path(), &child_meta, summary, scheduler)?;
|
||||
}
|
||||
scheduler(path)?;
|
||||
summary.dirs = summary.dirs.saturating_add(1);
|
||||
} else {
|
||||
scheduler(path)?;
|
||||
summary.files = summary.files.saturating_add(1);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn schedule_one(path: &Path) -> io::Result<()> {
|
||||
// `MoveFileExW + MOVEFILE_DELAY_UNTIL_REBOOT` requires absolute paths —
|
||||
// the session manager runs at boot before any working directory is
|
||||
// established, so a relative path cannot be resolved. The call sites
|
||||
// in `reset_local_data` already resolve paths via the core's
|
||||
// `config_get_data_paths` RPC (which returns absolute paths) so this
|
||||
// is currently a no-op in release builds; the assert catches a future
|
||||
// regression that wires a different caller in without thinking.
|
||||
debug_assert!(
|
||||
path.is_absolute(),
|
||||
"MoveFileExW + DELAY_UNTIL_REBOOT requires an absolute path, got {}",
|
||||
path.display()
|
||||
);
|
||||
let wide: Vec<u16> = path
|
||||
.as_os_str()
|
||||
.encode_wide()
|
||||
.chain(std::iter::once(0))
|
||||
.collect();
|
||||
// SAFETY: `wide` is a NUL-terminated UTF-16 buffer that outlives the
|
||||
// call. The destination pointer is `NULL`, which (combined with
|
||||
// `MOVEFILE_DELAY_UNTIL_REBOOT`) tells Windows to delete (rather than
|
||||
// rename) the source at the next boot. `MoveFileExW` returns BOOL —
|
||||
// non-zero on success.
|
||||
let ok = unsafe { MoveFileExW(wide.as_ptr(), std::ptr::null(), MOVEFILE_DELAY_UNTIL_REBOOT) };
|
||||
if ok == 0 {
|
||||
Err(io::Error::last_os_error())
|
||||
} else {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// Test-only no-op scheduler. Lets the traversal/counting tests run
|
||||
/// in any user context (incl. non-administrator) by sidestepping the
|
||||
/// `MoveFileExW + MOVEFILE_DELAY_UNTIL_REBOOT` call that would
|
||||
/// otherwise need HKLM write access. We capture the call order so a
|
||||
/// regression in depth-first ordering would surface as a wrong path
|
||||
/// sequence here, even though the real OS-side scheduling stays
|
||||
/// out of the test process.
|
||||
fn noop_scheduler(
|
||||
captured: &mut Vec<std::path::PathBuf>,
|
||||
) -> impl FnMut(&Path) -> io::Result<()> + '_ {
|
||||
move |path: &Path| {
|
||||
captured.push(path.to_path_buf());
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn schedule_walks_files_then_dirs() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let root = dir.path().join("reset-target");
|
||||
std::fs::create_dir_all(root.join("nested")).expect("mkdir nested");
|
||||
std::fs::write(root.join("a.txt"), b"a").expect("write a.txt");
|
||||
std::fs::write(root.join("nested").join("b.txt"), b"b").expect("write b.txt");
|
||||
|
||||
let mut captured = Vec::new();
|
||||
let summary =
|
||||
schedule_path_with_scheduler(&root, &mut noop_scheduler(&mut captured)).expect("walk");
|
||||
|
||||
// root + nested == 2 dirs; a.txt + nested/b.txt == 2 files
|
||||
assert_eq!(summary.files, 2, "expected 2 files queued, got {summary:?}");
|
||||
assert_eq!(summary.dirs, 2, "expected 2 dirs queued, got {summary:?}");
|
||||
assert_eq!(summary.total(), 4);
|
||||
assert!(!summary.partial, "Ok must not flag partial");
|
||||
|
||||
// Depth-first: a parent must only appear after all of its children.
|
||||
// Track per-path positions in the call order, then assert each
|
||||
// directory sits after every entry whose path is rooted inside it.
|
||||
let position = |needle: &Path| -> usize {
|
||||
captured
|
||||
.iter()
|
||||
.position(|p| p == needle)
|
||||
.unwrap_or_else(|| panic!("missing {} in {captured:?}", needle.display()))
|
||||
};
|
||||
let root_pos = position(&root);
|
||||
let nested_pos = position(&root.join("nested"));
|
||||
let a_pos = position(&root.join("a.txt"));
|
||||
let b_pos = position(&root.join("nested").join("b.txt"));
|
||||
assert!(
|
||||
b_pos < nested_pos,
|
||||
"b.txt must be scheduled before its parent (nested)"
|
||||
);
|
||||
assert!(
|
||||
nested_pos < root_pos && a_pos < root_pos,
|
||||
"nested + a.txt must be scheduled before root"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn schedule_single_file_reports_one_file() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let file = dir.path().join("solo.txt");
|
||||
std::fs::write(&file, b"x").expect("write solo.txt");
|
||||
|
||||
let mut captured = Vec::new();
|
||||
let summary =
|
||||
schedule_path_with_scheduler(&file, &mut noop_scheduler(&mut captured)).expect("walk");
|
||||
|
||||
assert_eq!(
|
||||
summary,
|
||||
RebootDeletionSchedule {
|
||||
files: 1,
|
||||
dirs: 0,
|
||||
partial: false,
|
||||
}
|
||||
);
|
||||
assert_eq!(captured, vec![file]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn schedule_missing_path_yields_not_found() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let missing = dir.path().join("does-not-exist");
|
||||
|
||||
let mut captured = Vec::new();
|
||||
let failure = schedule_path_with_scheduler(&missing, &mut noop_scheduler(&mut captured))
|
||||
.expect_err("missing");
|
||||
assert_eq!(failure.error.kind(), io::ErrorKind::NotFound);
|
||||
// Nothing scheduled, but partial flag still reports "did not
|
||||
// complete" so callers can distinguish from a clean success.
|
||||
assert!(failure.partial.partial);
|
||||
assert_eq!(failure.partial.files, 0);
|
||||
assert_eq!(failure.partial.dirs, 0);
|
||||
assert!(
|
||||
captured.is_empty(),
|
||||
"no scheduling should have been attempted: {captured:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn schedule_empty_dir_counts_one_dir() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let empty = dir.path().join("empty-target");
|
||||
std::fs::create_dir(&empty).expect("mkdir empty-target");
|
||||
|
||||
let mut captured = Vec::new();
|
||||
let summary =
|
||||
schedule_path_with_scheduler(&empty, &mut noop_scheduler(&mut captured)).expect("walk");
|
||||
|
||||
assert_eq!(
|
||||
summary,
|
||||
RebootDeletionSchedule {
|
||||
files: 0,
|
||||
dirs: 1,
|
||||
partial: false,
|
||||
}
|
||||
);
|
||||
assert_eq!(captured, vec![empty]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn schedule_propagates_scheduler_failure_with_partial_counts() {
|
||||
// Simulate the non-administrator MoveFileExW failure path: the
|
||||
// walk visits children successfully, then the third call (the
|
||||
// parent dir) errors out. The Err must carry the leaf counts
|
||||
// queued before the failure so the caller can surface "we did
|
||||
// get X files scheduled before this hit the registry wall."
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let root = dir.path().join("partial-fail");
|
||||
std::fs::create_dir_all(&root).expect("mkdir");
|
||||
std::fs::write(root.join("a.txt"), b"a").expect("write a.txt");
|
||||
std::fs::write(root.join("b.txt"), b"b").expect("write b.txt");
|
||||
|
||||
let root_path = root.clone();
|
||||
let mut count = 0usize;
|
||||
let mut scheduler = |path: &Path| -> io::Result<()> {
|
||||
count += 1;
|
||||
if path == root_path {
|
||||
Err(io::Error::new(
|
||||
io::ErrorKind::PermissionDenied,
|
||||
"simulated non-admin MoveFileExW failure",
|
||||
))
|
||||
} else {
|
||||
Ok(())
|
||||
}
|
||||
};
|
||||
|
||||
let failure =
|
||||
schedule_path_with_scheduler(&root, &mut scheduler).expect_err("scheduler failed");
|
||||
assert_eq!(failure.error.kind(), io::ErrorKind::PermissionDenied);
|
||||
assert!(failure.partial.partial);
|
||||
// Both leaf files were scheduled before the parent-dir call failed.
|
||||
assert_eq!(failure.partial.files, 2, "got {:?}", failure.partial);
|
||||
assert_eq!(failure.partial.dirs, 0, "got {:?}", failure.partial);
|
||||
// Sanity: scheduler was called for both leaves + the parent.
|
||||
assert_eq!(count, 3);
|
||||
}
|
||||
}
|
||||
+95
-6
@@ -14,7 +14,7 @@
|
||||
use std::fmt;
|
||||
use std::io::{self, IsTerminal};
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::{Once, OnceLock};
|
||||
use std::sync::{Mutex, Once, OnceLock};
|
||||
|
||||
use nu_ansi_term::{Color, Style};
|
||||
use tracing::{Event, Level};
|
||||
@@ -28,10 +28,19 @@ use tracing_subscriber::Layer;
|
||||
|
||||
static INIT: Once = Once::new();
|
||||
|
||||
/// Holds the non-blocking writer guard for the file appender. Must live for
|
||||
/// the entire process lifetime — dropping it stops the background flushing
|
||||
/// thread and silently swallows pending log records.
|
||||
static FILE_GUARD: OnceLock<WorkerGuard> = OnceLock::new();
|
||||
/// Holds the non-blocking writer guard for the file appender. Dropping it
|
||||
/// stops the background flushing thread and releases the OS file handle on
|
||||
/// the active `openhuman-YYYY-MM-DD.log`, which on Windows is required
|
||||
/// before the parent `<data_dir>/logs/` directory can be removed (issue
|
||||
/// #1615 — the file is held by the Tauri host process, not the embedded
|
||||
/// core, so `CoreProcessHandle::shutdown` alone does not release it).
|
||||
///
|
||||
/// Wrapped in `Mutex<Option<_>>` instead of `OnceLock` so [`shutdown_file_guard`]
|
||||
/// can `take` and drop the guard mid-process during `reset_local_data`.
|
||||
/// After a `take`, the file layer's writer becomes a no-op (the background
|
||||
/// thread has exited); see [`shutdown_file_guard`] docs for the consequence
|
||||
/// on subsequent log records.
|
||||
static FILE_GUARD: Mutex<Option<WorkerGuard>> = Mutex::new(None);
|
||||
|
||||
/// Resolved path to the active log file directory. Populated by
|
||||
/// [`init_for_embedded`] so UI commands (e.g. `reveal_logs_folder`) can find
|
||||
@@ -289,7 +298,9 @@ pub fn init_for_embedded(data_dir: &Path, verbose: bool) {
|
||||
{
|
||||
Ok(()) => {
|
||||
if let Some((_, guard, dir)) = pending_file {
|
||||
let _ = FILE_GUARD.set(guard);
|
||||
if let Ok(mut slot) = FILE_GUARD.lock() {
|
||||
*slot = Some(guard);
|
||||
}
|
||||
let _ = LOG_DIR.set(dir);
|
||||
}
|
||||
}
|
||||
@@ -315,6 +326,37 @@ pub fn log_directory() -> Option<&'static Path> {
|
||||
LOG_DIR.get().map(PathBuf::as_path)
|
||||
}
|
||||
|
||||
/// Drop the file appender's worker guard so the rolling `openhuman-*.log`
|
||||
/// file handle held by *this* process is released.
|
||||
///
|
||||
/// Returns `true` if a guard was taken (and dropped here), `false` if no
|
||||
/// guard was installed (CLI run, init failed, or already shut down). After
|
||||
/// this call:
|
||||
///
|
||||
/// * the non-blocking writer's background thread exits as part of `drop`,
|
||||
/// * the OS file handle on today's log file is closed,
|
||||
/// * subsequent `tracing::*` records routed to the file layer are silently
|
||||
/// discarded (the writer becomes a no-op) until the process restarts.
|
||||
///
|
||||
/// **Used by**: the Tauri shell's `reset_local_data` command, which must be
|
||||
/// able to `remove_dir_all(<data_dir>)` on Windows. Without releasing this
|
||||
/// guard the host process holds an open handle inside `<data_dir>/logs/`
|
||||
/// and Windows returns `ERROR_SHARING_VIOLATION` (os error 32). The stderr
|
||||
/// and Sentry layers stay attached because they don't keep files open.
|
||||
///
|
||||
/// **Not idempotent in the recover-after-call sense**: there is no re-init
|
||||
/// path. A subsequent `init_for_embedded` is a no-op (the `Once` guard has
|
||||
/// already fired), so file logging stays off until the next process launch.
|
||||
/// `reset_local_data` is followed by `ensure_running()` which restarts the
|
||||
/// embedded core but does *not* re-install the subscriber — by design, the
|
||||
/// user is expected to restart the app shortly after a reset.
|
||||
pub fn shutdown_file_guard() -> bool {
|
||||
let Ok(mut slot) = FILE_GUARD.lock() else {
|
||||
return false;
|
||||
};
|
||||
slot.take().is_some()
|
||||
}
|
||||
|
||||
fn seed_rust_log(verbose: bool, default_scope: CliLogDefault) {
|
||||
if std::env::var_os("RUST_LOG").is_some() {
|
||||
return;
|
||||
@@ -378,6 +420,14 @@ mod tests {
|
||||
/// concurrent env-var writes would race.
|
||||
static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
|
||||
|
||||
/// Serialize tests that mutate the process-global `FILE_GUARD` static.
|
||||
/// Without this, `shutdown_file_guard_takes_installed_guard` can race
|
||||
/// any concurrent test that calls `init_for_embedded` (or that itself
|
||||
/// stashes / takes the guard), making one of them observe a guard it
|
||||
/// did not install. Mirror of the `SCHEDULE_LOCK` pattern in
|
||||
/// `app/src-tauri/src/reset_reboot_schedule.rs::tests`.
|
||||
static FILE_GUARD_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
|
||||
|
||||
fn with_clean_rust_log<R>(f: impl FnOnce() -> R) -> R {
|
||||
let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
let prior = std::env::var("RUST_LOG").ok();
|
||||
@@ -488,4 +538,43 @@ mod tests {
|
||||
assert!(log_directory().is_none());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shutdown_file_guard_takes_installed_guard() {
|
||||
let _g = FILE_GUARD_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
|
||||
// Simulate `init_for_embedded` having stashed a writer guard, then
|
||||
// assert that `shutdown_file_guard` empties the slot and reports
|
||||
// truthfully.
|
||||
let dir = tempfile::tempdir().expect("tempdir for guard test");
|
||||
let appender = tracing_appender::rolling::Builder::new()
|
||||
.rotation(tracing_appender::rolling::Rotation::DAILY)
|
||||
.filename_prefix("guard-test")
|
||||
.filename_suffix("log")
|
||||
.build(dir.path())
|
||||
.expect("rolling appender for guard test");
|
||||
let (_writer, guard) = tracing_appender::non_blocking(appender);
|
||||
{
|
||||
let mut slot = FILE_GUARD.lock().expect("file guard mutex poisoned");
|
||||
// Save any pre-existing guard (a prior test in this process may
|
||||
// have installed one) and restore it after the assertion runs.
|
||||
let prior = slot.replace(guard);
|
||||
drop(slot);
|
||||
|
||||
assert!(
|
||||
shutdown_file_guard(),
|
||||
"expected shutdown_file_guard to take the installed guard"
|
||||
);
|
||||
assert!(
|
||||
!shutdown_file_guard(),
|
||||
"second call must be a no-op when the slot is already empty"
|
||||
);
|
||||
|
||||
// Restore the prior guard so unrelated tests that depend on
|
||||
// `init_for_embedded` having installed one are not surprised.
|
||||
if let Ok(mut slot) = FILE_GUARD.lock() {
|
||||
*slot = prior;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user