diff --git a/packages/oneclient_app/src/hooks/actions.rs b/packages/oneclient_app/src/hooks/actions.rs index aee585fc..fb2e820c 100644 --- a/packages/oneclient_app/src/hooks/actions.rs +++ b/packages/oneclient_app/src/hooks/actions.rs @@ -439,11 +439,13 @@ impl Actions { let events = state.services.events.clone(); match state.java.install_runtime_from(&vendor, major).await { Ok(_) => events.signal(oneclient_events::Signal::JavaChanged), - Err(err) => events - .notify("Java install failed") - .body(err.to_string()) - .error() - .send(), + Err(err) => { + events + .notify("Java install failed") + .body(err.to_string()) + .error() + .send() + }, } }); } diff --git a/packages/oneclient_app/src/view/app/settings/java.rs b/packages/oneclient_app/src/view/app/settings/java.rs index ab0cbf31..e1c4309e 100644 --- a/packages/oneclient_app/src/view/app/settings/java.rs +++ b/packages/oneclient_app/src/view/app/settings/java.rs @@ -1,8 +1,10 @@ +use std::path::Path; + use freya::prelude::*; -use oneclient_java::{JavaRuntime, JavaVendor}; +use oneclient_java::{JavaRuntime, JavaVendor, is_launcher_managed}; use super::settings_page; -use crate::components::{Button, Icon, IconType, JavaInstallManager, ScrollArea}; +use crate::components::{Button, Icon, IconType, JavaInstallManager, OverlayPopup, ScrollArea}; use crate::hooks::{Actions, java_runtimes, use_dispatch, use_java_runtimes}; use crate::theme::colors; use crate::ui::border_all_color; @@ -14,15 +16,17 @@ pub struct SettingsJava; impl Component for SettingsJava { fn render(&self) -> impl IntoElement { let dispatch = use_dispatch(); + let removing_dispatch = dispatch.clone(); let runtimes_query = use_java_runtimes(); let runtimes = java_runtimes(&runtimes_query); let mut show_manager = use_state(|| false); + let pending_remove = use_state(|| None::); let mut shell = settings_page() .child(section_header("ADD RUNTIME")) .child(AddRow { show_manager }.into_element()) .child(section_header("INSTALLED RUNTIMES")) - .child(runtimes_table(runtimes)); + .child(runtimes_table(runtimes, pending_remove)); if *show_manager.read() { shell = shell.child( @@ -36,6 +40,11 @@ impl Component for SettingsJava { ); } + let pending = pending_remove.read().clone(); + if let Some(target) = pending { + shell = shell.child(confirm_remove_modal(removing_dispatch, pending_remove, target)) + } + shell.into_element() } } @@ -84,7 +93,10 @@ impl Component for AddRow { } } -fn runtimes_table(runtimes: Vec) -> impl IntoElement { +fn runtimes_table( + runtimes: Vec, + pending_remove: State>, +) -> impl IntoElement { if runtimes.is_empty() { return rect() .width(Size::fill()) @@ -115,6 +127,7 @@ fn runtimes_table(runtimes: Vec) -> impl IntoElement { RuntimeRow { runtime, last: idx + 1 == count, + pending_remove } .into_element(), ); @@ -164,11 +177,11 @@ fn table_header() -> impl IntoElement { struct RuntimeRow { runtime: JavaRuntime, last: bool, + pending_remove: State> } impl Component for RuntimeRow { fn render(&self) -> impl IntoElement { - let dispatch = use_dispatch(); let runtime = &self.runtime; let path = runtime.absolute_path.clone(); @@ -226,7 +239,7 @@ impl Component for RuntimeRow { ), ), ) - .child(remove_button(dispatch, path)) + .child(remove_button(self.pending_remove, path)) } } @@ -234,11 +247,110 @@ fn path_content_width(path: &str) -> f32 { (path.chars().count() as f32 * 7.0).max(1.0) } -fn remove_button(dispatch: Actions, path: String) -> impl IntoElement { +/// `managed` is settled when the row is clicked rather than while rendering so +/// the modal never touches the filesystem mid-frame +#[derive(Clone, PartialEq)] +struct PendingRemove { + path: String, + /// Whether the files live in OneClient's own java dir, which is the only + /// case where removal takes them off disk + managed: bool, +} + +fn confirm_remove_modal( + dispatch: Actions, + mut pending: State>, + target: PendingRemove, +) -> impl IntoElement { + let PendingRemove { path, managed } = target; + let remove_path = path.clone(); + + let consequence = if managed { + "OneClient installed this runtime, so removing it deletes its files from disk." + } else { + "You added this runtime from your own folder, so only the list entry goes. The files stay where they are." + }; + + let confirm = if managed { + Button::new().danger().text("Remove and delete files") + } else { + Button::new().primary().text("Remove from list") + }; + + OverlayPopup::new() + .on_close(move |()| pending.set(None)) + .child( + rect() + .width(Size::window_percent(100.)) + .height(Size::window_percent(100.)) + .center() + .child( + rect() + .vertical() + .width(Size::px(440.)) + .max_width(Size::window_percent(90.)) + .spacing(14.) + .padding(Gaps::new_all(20.)) + .corner_radius(CornerRadius::new_all(14.)) + .background(colors::page_elevated()) + .child( + label() + .text("Remove Java Runtime?") + .font_size(16.) + .font_weight(FontWeight::SEMI_BOLD) + .color(colors::fg_primary()), + ) + .child( + label() + .text(consequence) + .font_size(12.) + .max_lines(4) + .width(Size::fill()) + .color(colors::fg_secondary()), + ) + .child( + label() + .text(path) + .font_size(12.) + .max_lines(3) + .width(Size::fill()) + .color(colors::fg_secondary()), + ) + .child( + rect() + .horizontal() + .width(Size::fill()) + .main_align(Alignment::End) + .spacing(8.) + .child( + Button::new() + .secondary() + .on_press(move |_| pending.set(None)) + .text("Cancel"), + ) + .child(confirm.on_press(move |_| { + dispatch.remove_java_runtime(remove_path.clone()); + pending.set(None); + })), + ), + ) + ) + .into_element() +} + +fn remove_button( + mut pending_remove: State>, + path: String, +) -> impl IntoElement { Button::new() .ghost() .small() - .on_press(move |_| dispatch.remove_java_runtime(path.clone())) + .on_press(move |_| { + pending_remove.set(Some(PendingRemove { + managed: is_launcher_managed(Path::new(&path)), + path: path.clone(), + })); + }) .child( Icon::new(IconType::Trash01) .size(14.) diff --git a/packages/oneclient_core/src/state.rs b/packages/oneclient_core/src/state.rs index 7bda2d29..bb9f7e29 100644 --- a/packages/oneclient_core/src/state.rs +++ b/packages/oneclient_core/src/state.rs @@ -106,9 +106,27 @@ impl LauncherState { } } +/// Java archives are the only downloads big enough for a leaked scratch file to +/// matter and they land flat in the java dir so this needs no recursion +async fn sweep_java_scratch_files() { + let dir = match paths::java_dir() { + Ok(dir) => dir, + Err(err) => { + tracing::warn!("could not resolve the java dir to sweep: {err:#}"); + return; + } + }; + + if let Err(err) = polyio::sweep_temp_files(&dir).await { + tracing::warn!("java scratch file sweep failed: {err:#}"); + } +} + pub fn run_startup_tasks(state: &Arc) { let background = Arc::clone(state); tokio::spawn(async move { + sweep_java_scratch_files().await; + let recovery = match crate::recovery::reconstruct_from_disk(&background).await { Ok(report) => report, Err(err) => { diff --git a/packages/oneclient_java/src/install.rs b/packages/oneclient_java/src/install.rs index 1e81c076..08cd6914 100644 --- a/packages/oneclient_java/src/install.rs +++ b/packages/oneclient_java/src/install.rs @@ -91,6 +91,60 @@ pub async fn install_package( Ok(executable) } +/// The directory this crate extracted for `executable`, or `None` when the +/// runtime sits outside the launcher's java dir which is where a folder the +/// user added themselves points +#[tracing::instrument(level = "debug")] +fn managed_install_root(executable: &Path) -> JavaResult> { + let java_dir = paths::java_dir()?; + + let Ok(root) = polyio::canonicalize(&java_dir) else { + return Ok(None); + }; + + // Refuses to resolve once the executable is gone which is the safe answer + // there is no way left to prove what the path pointed at + let canon = match polyio::ensure_under(executable, [&root]) { + Ok(Some(canon)) => canon, + Ok(None) => return Ok(None), + Err(err) => { + tracing::debug!("could not resolve the java runtime path: {err}"); + return Ok(None); + } + }; + + let Ok(relative) = canon.strip_prefix(&root) else { + return Ok(None); + }; + + match relative.components().next() { + Some(std::path::Component::Normal(name)) => Ok(Some(root.join(name))), + _ => Ok(None), + } +} + +/// Whether [`remove_installed_package`] would take this runtime's files with +/// it so the UI can say up front what removing it costs +#[must_use] +pub fn is_launcher_managed(executable: &Path) -> bool { + managed_install_root(executable).is_ok_and(|root| root.is_some()) +} + +/// Only ever touches OneClient's own java dir a runtime the user added from +/// their own folder is left on disk untouched +#[tracing::instrument(level = "debug")] +pub async fn remove_installed_package(executable: &Path) -> JavaResult { + let Some(install_root) = managed_install_root(executable)? else { + tracing::debug!("java runtime is not launcher-managed leaving its files alone"); + return Ok(false); + }; + + polyio::remove_dir_all(&install_root).await?; + + tracing::info!(path = %install_root.display(), "removed installed Java runtime files"); + Ok(true) +} + #[tracing::instrument(level = "debug")] fn resolve_installed_executable(extract_root: &Path, package: &JavaPackage) -> PathBuf { let mut base_path = extract_root.to_path_buf(); @@ -162,3 +216,105 @@ fn stem_without_archive(name: &str) -> String { stem.to_string() } } + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::atomic::{AtomicU32, Ordering}; + + /// Keeps the whole suite inside a temp tree the override is a `OnceLock` so + /// the first caller wins and every case works under one java dir + fn java_dir() -> PathBuf { + static ONCE: std::sync::Once = std::sync::Once::new(); + ONCE.call_once(|| { + paths::set_launcher_dir( + std::env::temp_dir().join(format!("oneclient-java-test-{}", std::process::id())), + ); + }); + + let dir = paths::java_dir().unwrap(); + std::fs::create_dir_all(&dir).unwrap(); + dir + } + + fn install_runtime(root: &Path) -> (PathBuf, PathBuf) { + static N: AtomicU32 = AtomicU32::new(0); + let install = root.join(format!("zulu21-{}", N.fetch_add(1, Ordering::Relaxed))); + let bin = install.join("zulu21.0.12").join("bin"); + std::fs::create_dir_all(&bin).unwrap(); + + let executable = bin.join("java"); + std::fs::write(&executable, b"").unwrap(); + (install, executable) + } + + #[test] + fn resolves_the_extracted_directory_not_the_executable_parent() { + let java_dir = java_dir(); + let (install, executable) = install_runtime(&java_dir); + + let resolved = managed_install_root(&executable).unwrap().unwrap(); + + assert_eq!(resolved, polyio::canonicalize(&install).unwrap()); + } + + #[tokio::test] + async fn removing_a_managed_runtime_deletes_its_files() { + let java_dir = java_dir(); + let (install, executable) = install_runtime(&java_dir); + + assert!(remove_installed_package(&executable).await.unwrap()); + + assert!(!install.exists()); + assert!(java_dir.exists(), "the java dir itself must survive"); + } + + #[tokio::test] + async fn a_runtime_outside_the_java_dir_keeps_its_files() { + let java_dir = java_dir(); + let elsewhere = java_dir.parent().unwrap().join("user-picked-jdk"); + let (_, executable) = install_runtime(&elsewhere); + + assert_eq!(managed_install_root(&executable).unwrap(), None); + assert!(!remove_installed_package(&executable).await.unwrap()); + assert!(executable.exists(), "a folder the user added is not ours to delete"); + + std::fs::remove_dir_all(&elsewhere).unwrap(); + } + + #[test] + fn a_traversal_back_out_of_the_java_dir_is_not_managed() { + let java_dir = java_dir(); + let elsewhere = java_dir.parent().unwrap().join("traversal-jdk"); + let (_, executable) = install_runtime(&elsewhere); + + let sneaky = java_dir + .join("..") + .join("traversal-jdk") + .join(executable.strip_prefix(&elsewhere).unwrap()); + + assert_eq!(managed_install_root(&sneaky).unwrap(), None); + + std::fs::remove_dir_all(&elsewhere).unwrap(); + } + + #[test] + fn the_managed_flag_matches_what_removal_would_actually_delete() { + let java_dir = java_dir(); + let (_, ours) = install_runtime(&java_dir); + let elsewhere = java_dir.parent().unwrap().join("flagged-jdk"); + let (_, theirs) = install_runtime(&elsewhere); + + assert!(is_launcher_managed(&ours)); + assert!(!is_launcher_managed(&theirs)); + + std::fs::remove_dir_all(&elsewhere).unwrap(); + } + + #[test] + fn the_java_dir_itself_is_never_managed() { + let java_dir = java_dir(); + + assert_eq!(managed_install_root(&java_dir).unwrap(), None); + } +} diff --git a/packages/oneclient_java/src/lib.rs b/packages/oneclient_java/src/lib.rs index 490ff736..847ea4cf 100644 --- a/packages/oneclient_java/src/lib.rs +++ b/packages/oneclient_java/src/lib.rs @@ -16,7 +16,7 @@ pub mod vendors; pub use checker::{JavaCheckInfo, check_java_runtime}; pub use data::{JavaPackage, JavaRuntime, PackageArchive, java_executable_relative_path}; pub use error::{JavaError, JavaResult}; -pub use install::install_package; +pub use install::{install_package, is_launcher_managed}; pub use locate::{LocatedJava, best_for_major, locate_java}; pub use platform::{HostArch, HostOs, HostTarget}; pub use service::{ diff --git a/packages/oneclient_java/src/service.rs b/packages/oneclient_java/src/service.rs index 37ba938c..abab472a 100644 --- a/packages/oneclient_java/src/service.rs +++ b/packages/oneclient_java/src/service.rs @@ -155,7 +155,17 @@ impl JavaService { #[tracing::instrument(skip(self))] pub async fn remove_runtime(&self, absolute_path: &str) -> JavaResult<()> { self.store.delete_by_path(absolute_path).await?; - tracing::info!("removed Java runtime"); + + let removed_files = + match crate::install::remove_installed_package(Path::new(absolute_path)).await { + Ok(removed) => removed, + Err(err) => { + tracing::warn!("could not remove the installed Java files: {err:#}"); + false + } + }; + + tracing::info!(removed_files, "removed Java runtime"); Ok(()) } diff --git a/packages/polyio/src/file.rs b/packages/polyio/src/file.rs index 0797a20f..09aed9b6 100644 --- a/packages/polyio/src/file.rs +++ b/packages/polyio/src/file.rs @@ -1,4 +1,5 @@ use std::sync::atomic::{AtomicU64, Ordering}; +use std::time::Duration; use std::{fs::Metadata, path::{Path, PathBuf}}; use async_tempfile::{TempDir, TempFile}; @@ -318,8 +319,94 @@ fn temp_sibling(path: &Path) -> PathBuf { } } -/// Readers see either the old contents or the complete new ones -/// The fsync before the rename stops a crash leaving a correctly-named zero-length file +// Time for a file to be flagged as abandoned +const STALE_TEMP_AGE: Duration = Duration::from_secs(15 * 60); + +fn temp_sibling_pid(name: &str) -> Option { + let inner = name.strip_prefix('.')?.strip_suffix(".tmp")?; + let (head, counter) = inner.rsplit_once('.')?; + let (stem, pid) = head.rsplit_once('.')?; + + counter.parse::().ok()?; + if stem.is_empty() { + return None; + } + + pid.parse().ok() +} + +/// Returns the number of bytes reclaimed non-recursive and never an error for a +/// missing directory +#[tracing::instrument( + level = "debug", + skip(dir), + fields(dir = %dir.as_ref().display()) +)] +pub async fn sweep_temp_files(dir: impl AsRef) -> PolyIOResult { + let dir = dir.as_ref(); + let path_err = |source| IOError::PathIOError { + source, + path: dir.to_string_lossy().to_string(), + }; + + let mut entries = match tokio::fs::read_dir(dir).await { + Ok(entries) => entries, + // Nothing has ever been written here so there is nothing to sweep + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(0), + Err(err) => return Err(path_err(err)), + }; + + let current_pid = std::process::id(); + let mut files = 0usize; + let mut bytes = 0u64; + + while let Some(entry) = entries.next_entry().await.map_err(path_err)? { + let name = entry.file_name(); + let Some(pid) = name.to_str().and_then(temp_sibling_pid) else { + continue; + }; + + if pid == current_pid { + continue; + } + + let Ok(metadata) = entry.metadata().await else { + continue; + }; + if !metadata.is_file() { + continue; + } + + let stale = metadata + .modified() + .ok() + .and_then(|modified| modified.elapsed().ok()) + .is_some_and(|age| age >= STALE_TEMP_AGE); + if !stale { + continue; + } + + let size = metadata.len(); + match tokio::fs::remove_file(entry.path()).await { + Ok(()) => { + files += 1; + bytes += size; + } + // Another process may have just renamed it out from under us + Err(err) => tracing::debug!( + path = %entry.path().display(), + "could not remove stale scratch file: {err}" + ), + } + } + + if files > 0 { + tracing::info!(files, bytes, dir = %dir.display(), "removed stale scratch files"); + } + + Ok(bytes) +} + #[tracing::instrument( level = "debug", skip(path, data), @@ -383,10 +470,6 @@ pub async fn write_json_atomic( write_atomic(path, bytes).await } -/// Traversal guard for externally supplied paths -/// `Ok(None)` when the path resolves outside every root -/// `Err` only when `path` cannot be canonicalised -/// Roots that cannot be canonicalised (e.g. not yet created) are skipped #[tracing::instrument( level = "debug", skip(path, roots), @@ -396,8 +479,6 @@ pub fn ensure_under>( path: impl AsRef, roots: impl IntoIterator, ) -> PolyIOResult> { - // Not `std::fs::canonicalize` its Windows `\\?\` UNC output would never - // `starts_with` the plain-path roots so everything would look like an escape let canon = crate::canonicalize(path)?; for root in roots { @@ -411,9 +492,6 @@ pub fn ensure_under>( Ok(None) } -/// `exclude_top` applies only at the top level a nested directory of the same -/// name is still copied -/// Symlinks are followed and copied as their contents #[tracing::instrument(level = "debug", skip(exclude_top))] pub async fn copy_dir(src: &Path, dst: &Path, exclude_top: &[&str]) -> PolyIOResult<()> { let mut stack: Vec<(PathBuf, PathBuf, bool)> = @@ -723,6 +801,69 @@ mod tests { std::fs::remove_dir_all(&dir).unwrap(); } + /// Ages a file past [`STALE_TEMP_AGE`] so the sweep treats it as abandoned + fn backdate(path: &Path) { + let stale = std::time::SystemTime::now() - (STALE_TEMP_AGE + Duration::from_secs(60)); + let file = std::fs::File::options().write(true).open(path).unwrap(); + file.set_times(std::fs::FileTimes::new().set_modified(stale)) + .unwrap(); + } + + #[test] + fn temp_sibling_names_round_trip() { + let name = temp_sibling(Path::new("/java/zulu21.zip")); + let name = name.file_name().unwrap().to_str().unwrap(); + + assert_eq!(temp_sibling_pid(name), Some(std::process::id())); + + // Files polyio did not write + assert_eq!(temp_sibling_pid("zulu21.zip"), None); + assert_eq!(temp_sibling_pid(".vimrc.tmp"), None); + assert_eq!(temp_sibling_pid(".cache.notapid.7.tmp"), None); + assert_eq!(temp_sibling_pid(".cache.4242.notacounter.tmp"), None); + } + + #[tokio::test] + async fn sweep_removes_only_abandoned_scratch_files() { + let dir = scratch("sweep"); + + let abandoned = dir.join(format!(".zulu21.zip.{}.0.tmp", std::process::id() + 1)); + std::fs::write(&abandoned, b"half a runtime").unwrap(); + backdate(&abandoned); + + // Same shape but young enough to still have a writer behind it + let in_flight = dir.join(format!(".zulu17.zip.{}.0.tmp", std::process::id() + 2)); + std::fs::write(&in_flight, b"downloading").unwrap(); + + // Ours, however old the launcher has been up + let ours = temp_sibling(&dir.join("zulu8.zip")); + std::fs::write(&ours, b"mine").unwrap(); + backdate(&ours); + + // Not a scratch file at all + let keep = dir.join("zulu21.zip"); + std::fs::write(&keep, b"a real archive").unwrap(); + backdate(&keep); + + let freed = sweep_temp_files(&dir).await.unwrap(); + + assert_eq!(freed, "half a runtime".len() as u64); + assert!(!abandoned.exists()); + assert!(in_flight.exists()); + assert!(ours.exists()); + assert!(keep.exists()); + + std::fs::remove_dir_all(&dir).unwrap(); + } + + #[tokio::test] + async fn sweep_ignores_a_missing_directory() { + let dir = scratch("sweep-missing"); + std::fs::remove_dir_all(&dir).unwrap(); + + assert_eq!(sweep_temp_files(&dir).await.unwrap(), 0); + } + fn failing_stream( chunks: Vec<&'static [u8]>, ) -> impl futures_lite::Stream> + Unpin + Send {