diff --git a/desktop/src/cef.rs b/desktop/src/cef.rs index b9649e492d..e3cc85405f 100644 --- a/desktop/src/cef.rs +++ b/desktop/src/cef.rs @@ -27,7 +27,6 @@ use crate::wrapper::{WgpuContext, deserialize_editor_message}; mod consts; mod context; -mod dirs; mod input; mod internal; mod ipc; diff --git a/desktop/src/cef/context/builder.rs b/desktop/src/cef/context/builder.rs index c47fae6c83..4d79ec55d9 100644 --- a/desktop/src/cef/context/builder.rs +++ b/desktop/src/cef/context/builder.rs @@ -4,15 +4,15 @@ use cef::{ App, BrowserSettings, CefString, Client, DictionaryValue, ImplCommandLine, ImplRequestContext, LogSeverity, RequestContextSettings, SchemeHandlerFactory, Settings, WindowInfo, api_hash, browser_host_create_browser_sync, execute_process, }; -use std::path::{Path, PathBuf}; +use std::path::Path; use super::CefContext; use super::singlethreaded::SingleThreadedCefContext; use crate::cef::CefEventHandler; use crate::cef::consts::{RESOURCE_DOMAIN, RESOURCE_SCHEME}; -use crate::cef::dirs::{create_instance_dir, delete_instance_dirs}; use crate::cef::input::InputState; use crate::cef::internal::{BrowserProcessAppImpl, BrowserProcessClientImpl, RenderProcessAppImpl, SchemeHandlerFactoryImpl}; +use crate::dirs::TempDir; pub(crate) struct CefContextBuilder { pub(crate) args: Args, @@ -97,8 +97,7 @@ impl CefContextBuilder { #[cfg(target_os = "macos")] pub(crate) fn initialize(self, event_handler: H, disable_gpu_acceleration: bool) -> Result { - delete_instance_dirs(); - let instance_dir = create_instance_dir(); + let instance_dir = TempDir::new().expect("Failed to create temporary directory for CEF instance"); let exe = std::env::current_exe().expect("cannot get current exe path"); let app_root = exe.parent().and_then(|p| p.parent()).expect("bad path structure").parent().expect("bad path structure"); @@ -108,7 +107,7 @@ impl CefContextBuilder { multi_threaded_message_loop: 0, external_message_pump: 1, no_sandbox: 1, // GPU helper crashes when running with sandbox - ..Self::common_settings(&instance_dir) + ..Self::common_settings(instance_dir.as_ref()) }; self.initialize_inner(&event_handler, settings)?; @@ -118,14 +117,13 @@ impl CefContextBuilder { #[cfg(not(target_os = "macos"))] pub(crate) fn initialize(self, event_handler: H, disable_gpu_acceleration: bool) -> Result { - delete_instance_dirs(); - let instance_dir = create_instance_dir(); + let instance_dir = TempDir::new().expect("Failed to create temporary directory for CEF instance"); let settings = Settings { multi_threaded_message_loop: 1, #[cfg(target_os = "linux")] no_sandbox: 1, - ..Self::common_settings(&instance_dir) + ..Self::common_settings(instance_dir.as_ref()) }; self.initialize_inner(&event_handler, settings)?; @@ -157,7 +155,7 @@ impl CefContextBuilder { } } -fn create_browser(event_handler: H, instance_dir: PathBuf, disable_gpu_acceleration: bool) -> Result { +fn create_browser(event_handler: H, instance_dir: TempDir, disable_gpu_acceleration: bool) -> Result { let mut client = Client::new(BrowserProcessClientImpl::new(&event_handler)); #[cfg(feature = "accelerated_paint")] @@ -211,7 +209,7 @@ fn create_browser(event_handler: H, instance_dir: PathBuf, d event_handler: Box::new(event_handler), browser, input_state: InputState::default(), - instance_dir, + _instance_dir: instance_dir, }) } else { tracing::error!("Failed to create browser"); diff --git a/desktop/src/cef/context/multithreaded.rs b/desktop/src/cef/context/multithreaded.rs index 239e734d22..832cfd2f2f 100644 --- a/desktop/src/cef/context/multithreaded.rs +++ b/desktop/src/cef/context/multithreaded.rs @@ -54,7 +54,12 @@ impl CefContext for MultiThreadedCefContextProxy { impl Drop for MultiThreadedCefContextProxy { fn drop(&mut self) { // Force dropping underlying context on the UI thread - run_on_ui_thread(move || drop(CONTEXT.take())); + let (sync_drop_tx, sync_drop_rx) = std::sync::mpsc::channel(); + run_on_ui_thread(move || { + drop(CONTEXT.take()); + let _ = sync_drop_tx.send(()); + }); + let _ = sync_drop_rx.recv(); } } diff --git a/desktop/src/cef/context/singlethreaded.rs b/desktop/src/cef/context/singlethreaded.rs index 33c3e64c19..fe4e01a8d2 100644 --- a/desktop/src/cef/context/singlethreaded.rs +++ b/desktop/src/cef/context/singlethreaded.rs @@ -4,6 +4,7 @@ use winit::event::WindowEvent; use crate::cef::input::InputState; use crate::cef::ipc::{MessageType, SendMessage}; use crate::cef::{CefEventHandler, input}; +use crate::dirs::TempDir; use super::CefContext; @@ -11,7 +12,7 @@ pub(super) struct SingleThreadedCefContext { pub(super) event_handler: Box, pub(super) browser: Browser, pub(super) input_state: InputState, - pub(super) instance_dir: std::path::PathBuf, + pub(super) _instance_dir: TempDir, } impl CefContext for SingleThreadedCefContext { @@ -46,19 +47,6 @@ impl Drop for SingleThreadedCefContext { // CEF wants us to close the browser before shutting down, otherwise it may run longer that necessary. self.browser.host().unwrap().close_browser(1); cef::shutdown(); - - // Sometimes some CEF processes still linger at this point and hold file handles to the cache directory. - // To mitigate this, we try to remove the directory multiple times with some delay. - // TODO: find a better solution if possible. - for _ in 0..30 { - match std::fs::remove_dir_all(&self.instance_dir) { - Ok(_) => break, - Err(e) => { - tracing::warn!("Failed to remove CEF cache directory, retrying...: {e}"); - std::thread::sleep(std::time::Duration::from_millis(100)); - } - } - } } } @@ -68,7 +56,6 @@ impl SendMessage for SingleThreadedCefContext { tracing::error!("Main frame is not available, cannot send message"); return; }; - frame.send_message(message_type, message); } } diff --git a/desktop/src/cef/dirs.rs b/desktop/src/cef/dirs.rs deleted file mode 100644 index 5046fc6e78..0000000000 --- a/desktop/src/cef/dirs.rs +++ /dev/null @@ -1,24 +0,0 @@ -use std::path::PathBuf; - -use crate::dirs::{app_data_dir, ensure_dir_exists}; - -static CEF_DIR_NAME: &str = "browser"; - -pub(crate) fn delete_instance_dirs() { - let cef_dir = app_data_dir().join(CEF_DIR_NAME); - if let Ok(entries) = std::fs::read_dir(&cef_dir) { - for entry in entries.flatten() { - let path = entry.path(); - if path.is_dir() { - let _ = std::fs::remove_dir_all(&path); - } - } - } -} - -pub(crate) fn create_instance_dir() -> PathBuf { - let instance_id: String = (0..32).map(|_| format!("{:x}", rand::random::() % 16)).collect(); - let path = app_data_dir().join(CEF_DIR_NAME).join(instance_id); - ensure_dir_exists(&path); - path -} diff --git a/desktop/src/dirs.rs b/desktop/src/dirs.rs index 584f1a290c..0ec18aa2f8 100644 --- a/desktop/src/dirs.rs +++ b/desktop/src/dirs.rs @@ -1,11 +1,31 @@ -use std::fs::create_dir_all; -use std::path::PathBuf; +use std::fs; +use std::io; +use std::path::{Path, PathBuf}; use crate::consts::{APP_DIRECTORY_NAME, APP_DOCUMENTS_DIRECTORY_NAME}; pub(crate) fn ensure_dir_exists(path: &PathBuf) { if !path.exists() { - create_dir_all(path).unwrap_or_else(|_| panic!("Failed to create directory at {path:?}")); + fs::create_dir_all(path).unwrap_or_else(|_| panic!("Failed to create directory at {path:?}")); + } +} + +fn clear_dir(path: &PathBuf) { + let Ok(entries) = fs::read_dir(path) else { + tracing::error!("Failed to read directory at {path:?}"); + return; + }; + for entry in entries.flatten() { + let entry_path = entry.path(); + if entry_path.is_dir() { + if let Err(e) = fs::remove_dir_all(&entry_path) { + tracing::error!("Failed to remove directory at {:?}: {}", entry_path, e); + } + } else if entry_path.is_file() { + if let Err(e) = fs::remove_file(&entry_path) { + tracing::error!("Failed to remove file at {:?}: {}", entry_path, e); + } + } } } @@ -15,8 +35,52 @@ pub(crate) fn app_data_dir() -> PathBuf { path } +fn app_tmp_dir() -> PathBuf { + let path = std::env::temp_dir().join(APP_DIRECTORY_NAME); + ensure_dir_exists(&path); + path +} + +pub(crate) fn app_tmp_dir_cleanup() { + clear_dir(&app_tmp_dir()); +} + pub(crate) fn app_autosave_documents_dir() -> PathBuf { let path = app_data_dir().join(APP_DOCUMENTS_DIRECTORY_NAME); ensure_dir_exists(&path); path } + +/// Temporary directory that is automatically deleted when dropped. +pub struct TempDir { + path: PathBuf, +} + +impl TempDir { + pub fn new() -> io::Result { + Self::new_with_parent(app_tmp_dir()) + } + + pub fn new_with_parent(parent: impl AsRef) -> io::Result { + let random_suffix = format!("{:032x}", rand::random::()); + let name = format!("{}_{}", std::process::id(), random_suffix); + let path = parent.as_ref().join(name); + fs::create_dir_all(&path)?; + Ok(Self { path }) + } +} + +impl Drop for TempDir { + fn drop(&mut self) { + let result = fs::remove_dir_all(&self.path); + if let Err(e) = result { + tracing::error!("Failed to remove temporary directory at {:?}: {}", self.path, e); + } + } +} + +impl AsRef for TempDir { + fn as_ref(&self) -> &Path { + &self.path + } +} diff --git a/desktop/src/lib.rs b/desktop/src/lib.rs index 8448c0e30a..fda880584f 100644 --- a/desktop/src/lib.rs +++ b/desktop/src/lib.rs @@ -63,6 +63,8 @@ pub fn start() { } }; + dirs::app_tmp_dir_cleanup(); + let prefs = preferences::read(); // Must be called before event loop initialization or native window integrations will break diff --git a/desktop/src/persist.rs b/desktop/src/persist.rs index 5c2f0e16ef..fb536bfae7 100644 --- a/desktop/src/persist.rs +++ b/desktop/src/persist.rs @@ -98,6 +98,8 @@ impl PersistentData { } pub(crate) fn load_from_disk(&mut self) { + delete_old_cef_browser_directory(); + let path = Self::state_file_path(); let data = match std::fs::read_to_string(&path) { Ok(d) => d, @@ -157,3 +159,11 @@ impl PersistentData { path } } + +// TODO: Eventually remove this cleanup code for the old "browser" CEF directory +fn delete_old_cef_browser_directory() { + let old_browser_dir = crate::dirs::app_data_dir().join("browser"); + if old_browser_dir.is_dir() { + let _ = std::fs::remove_dir_all(&old_browser_dir); + } +}