diff --git a/.changes/single-instance-release-on-cleanup.md b/.changes/single-instance-release-on-cleanup.md new file mode 100644 index 000000000..feb92fd87 --- /dev/null +++ b/.changes/single-instance-release-on-cleanup.md @@ -0,0 +1,5 @@ +--- +"single-instance": patch +--- + +Release the single instance lock in `AppHandle::cleanup_before_exit`, right before the process exits or restarts, instead of on `RunEvent::Exit`. The app now remains the single instance while your own `RunEvent::Exit` handler runs. `destroy` can still release the lock earlier. diff --git a/.github/workflows/test-rust.yml b/.github/workflows/test-rust.yml index e1726caf1..601dbd74d 100644 --- a/.github/workflows/test-rust.yml +++ b/.github/workflows/test-rust.yml @@ -176,10 +176,11 @@ jobs: # Only on the platforms we actually test on, since cargo skips doc tests when cross compiling. elif [ "$COMMAND" = test ] && ! $RUNNER test --doc --package "$package" --target "$TARGET" "${features[@]}"; then failed+=("$package (doc tests)") - # The D-Bus test is ignored by default because it needs its own session bus, and it - # is compiled out with `--all-features` (the `semver` feature changes the bus name). + # The D-Bus tests are ignored by default because they need their own session bus, and + # `dbus_errors` is compiled out with `--all-features` (the `semver` feature changes the + # bus name). `instance_lock` runs with the other tests on Windows and macOS. elif [ "$package" = tauri-plugin-single-instance ] && [ "$COMMAND" = test ] && [[ "$TARGET" == *-linux-* ]] \ - && ! dbus-run-session -- $RUNNER test --package "$package" --target "$TARGET" --test dbus_errors -- --ignored; then + && ! dbus-run-session -- $RUNNER test --package "$package" --target "$TARGET" --test dbus_errors --test instance_lock -- --ignored; then failed+=("$package (D-Bus tests)") fi echo "::endgroup::" diff --git a/plugins/single-instance/Cargo.toml b/plugins/single-instance/Cargo.toml index 7bf33daed..5a49edb56 100644 --- a/plugins/single-instance/Cargo.toml +++ b/plugins/single-instance/Cargo.toml @@ -41,6 +41,10 @@ features = [ "Win32_Graphics_Gdi", ] +[target."cfg(target_os = \"windows\")".dev-dependencies.windows-sys] +version = "0.61" +features = ["Win32_UI_WindowsAndMessaging"] + [target."cfg(target_os = \"linux\")".dependencies] zbus = { workspace = true } diff --git a/plugins/single-instance/src/lib.rs b/plugins/single-instance/src/lib.rs index a5c35c582..b4aa295d0 100644 --- a/plugins/single-instance/src/lib.rs +++ b/plugins/single-instance/src/lib.rs @@ -15,7 +15,7 @@ )] #![cfg(not(any(target_os = "android", target_os = "ios")))] -use tauri::{AppHandle, Manager, Runtime, plugin::TauriPlugin}; +use tauri::{AppHandle, Manager, Resource, ResourceId, Runtime, plugin::TauriPlugin}; #[cfg(target_os = "windows")] #[path = "platform_impl/windows.rs"] @@ -56,12 +56,27 @@ pub fn init, Vec, String) + Send + Sy /// Releases the resources this plugin uses to detect other instances (the named mutex on /// Windows, the D-Bus name on Linux or the Unix socket on macOS). /// -/// The plugin calls this automatically on [`tauri::RunEvent::Exit`], so you normally don't need -/// to call it yourself. Call it manually before terminating the process through means that skip -/// that event, such as [`std::process::exit`], so a future instance of the app isn't mistaken -/// for a still-running one. +/// The plugin releases them automatically in [`AppHandle::cleanup_before_exit`], which Tauri runs +/// right before the process exits or restarts (after your [`tauri::RunEvent::Exit`] handler +/// returns), so you normally don't need to call this yourself and the app keeps being the single +/// instance while your own exit cleanup runs. Call it manually to release the lock earlier, or +/// before terminating the process through means that skip [`AppHandle::cleanup_before_exit`], such +/// as [`std::process::exit`], so a future instance of the app isn't mistaken for a still-running +/// one. pub fn destroy>(manager: &M) { - platform_impl::destroy(manager) + if let Some(lock) = manager.try_state::() { + let _ = manager.resources_table().close(lock.0); + } +} + +struct InstanceLockId(ResourceId); + +/// Stores the platform lock in the app's resource table, which Tauri clears in +/// [`AppHandle::cleanup_before_exit`], so the lock's [`Drop`] implementation releases it only once +/// the app is really exiting. +pub(crate) fn manage_instance_lock>(manager: &M, lock: impl Resource) { + let rid = manager.resources_table().add(lock); + manager.manage(InstanceLockId(rid)); } /// Builds the single-instance plugin. diff --git a/plugins/single-instance/src/platform_impl/linux.rs b/plugins/single-instance/src/platform_impl/linux.rs index 833687c78..2e98ea5ed 100644 --- a/plugins/single-instance/src/platform_impl/linux.rs +++ b/plugins/single-instance/src/platform_impl/linux.rs @@ -5,14 +5,28 @@ #[cfg(feature = "semver")] use crate::semver_compat::semver_compat_string; -use crate::SingleInstanceCallback; +use crate::{SingleInstanceCallback, manage_instance_lock}; use tauri::{ - AppHandle, Manager, RunEvent, Runtime, + AppHandle, Resource, Runtime, plugin::{self, TauriPlugin}, }; use zbus::{blocking::Connection, interface, names::WellKnownName}; -struct ConnectionHandle(Connection); +/// Owns the D-Bus name, released when the app exits (see [`crate::destroy`]). +struct InstanceLock { + connection: Connection, + name: String, +} + +impl Resource for InstanceLock {} + +impl Drop for InstanceLock { + fn drop(&mut self) { + if let Ok(name) = WellKnownName::try_from(self.name.as_str()) { + let _ = self.connection.release_name(name); + } + } +} struct SingleInstanceDBus { callback: Box>, @@ -26,8 +40,6 @@ impl SingleInstanceDBus { } } -struct DBusName(String); - pub fn init( callback: Box>, dbus_id: Option, @@ -71,7 +83,13 @@ pub fn init( .build() { Ok(connection) => { - app.manage(ConnectionHandle(connection)); + manage_instance_lock( + app, + InstanceLock { + connection, + name: dbus_name, + }, + ); } Err(zbus::Error::NameTaken) => { let connection = Connection::session()?; @@ -107,24 +125,7 @@ pub fn init( } } - app.manage(DBusName(dbus_name)); - Ok(()) }) - .on_event(move |app, event| { - if let RunEvent::Exit = event { - destroy(app); - } - }) .build() } - -pub fn destroy>(manager: &M) { - if let Some(connection) = manager.try_state::() - && let Some(dbus_name) = manager - .try_state::() - .and_then(|name| WellKnownName::try_from(name.0.clone()).ok()) - { - let _ = connection.0.release_name(dbus_name); - } -} diff --git a/plugins/single-instance/src/platform_impl/macos.rs b/plugins/single-instance/src/platform_impl/macos.rs index c038ff251..00d19506b 100644 --- a/plugins/single-instance/src/platform_impl/macos.rs +++ b/plugins/single-instance/src/platform_impl/macos.rs @@ -8,15 +8,26 @@ use std::{ path::PathBuf, }; -use crate::SingleInstanceCallback; #[cfg(feature = "semver")] use crate::semver_compat::semver_compat_string; +use crate::{SingleInstanceCallback, manage_instance_lock}; use tauri::{ - AppHandle, Config, Manager, RunEvent, Runtime, + AppHandle, Config, Manager, Resource, Runtime, plugin::{self, TauriPlugin}, }; use tokio::io::AsyncReadExt; +/// Owns the Unix socket file, removed when the app exits (see [`crate::destroy`]). +struct InstanceLock(PathBuf); + +impl Resource for InstanceLock {} + +impl Drop for InstanceLock { + fn drop(&mut self) { + socket_cleanup(&self.0); + } +} + pub fn init(cb: Box>) -> TauriPlugin { plugin::Builder::new("single-instance") .setup(|app, _api| { @@ -32,6 +43,7 @@ pub fn init(cb: Box>) -> TauriPlugin { ErrorKind::NotFound | ErrorKind::ConnectionRefused => { // This process claims itself as singleton as likely none exists socket_cleanup(&socket); + manage_instance_lock(app, InstanceLock(socket.clone())); listen_for_other_instances(socket, app.clone(), cb); } _ => { @@ -45,19 +57,9 @@ pub fn init(cb: Box>) -> TauriPlugin { } Ok(()) }) - .on_event(|app, event| { - if let RunEvent::Exit = event { - destroy(app); - } - }) .build() } -pub fn destroy>(manager: &M) { - let socket = socket_path(manager.config(), manager.package_info()); - socket_cleanup(&socket); -} - fn socket_path(config: &Config, _package_info: &tauri::PackageInfo) -> PathBuf { let identifier = config.identifier.replace(['.', '-'].as_ref(), "_"); diff --git a/plugins/single-instance/src/platform_impl/windows.rs b/plugins/single-instance/src/platform_impl/windows.rs index 27a3581f3..10610d862 100644 --- a/plugins/single-instance/src/platform_impl/windows.rs +++ b/plugins/single-instance/src/platform_impl/windows.rs @@ -5,9 +5,9 @@ #[cfg(feature = "semver")] use crate::semver_compat::semver_compat_string; -use crate::SingleInstanceCallback; +use crate::{SingleInstanceCallback, manage_instance_lock}; use tauri::{ - AppHandle, Manager, RunEvent, Runtime, + AppHandle, Resource, Runtime, plugin::{self, TauriPlugin}, }; use windows_sys::Win32::{ @@ -36,9 +36,24 @@ const WMCOPYDATA_SINGLE_INSTANCE_DATA_NUL: usize = 1543; /// How long the second instance waits for the first instance to take its arguments. const FORWARD_TIMEOUT_MS: u32 = 10_000; -struct MutexHandle(isize); +/// Owns the named mutex and the event target window, released when the app exits +/// (see [`crate::destroy`]). +struct InstanceLock { + mutex: isize, + window: isize, +} -struct TargetWindowHandle(isize); +impl Resource for InstanceLock {} + +impl Drop for InstanceLock { + fn drop(&mut self) { + unsafe { + ReleaseMutex(self.mutex as _); + CloseHandle(self.mutex as _); + DestroyWindow(self.window as _); + } + } +} struct UserData { app: AppHandle, @@ -145,39 +160,26 @@ pub fn init(callback: Box>) -> TauriPlugin std::process::exit(0); } } else { - app.manage(MutexHandle(hmutex as _)); - let userdata = UserData { app: app.clone(), callback, }; let userdata = Box::into_raw(Box::new(userdata)); let hwnd = create_event_target_window::(&class_name, &window_name, userdata); - app.manage(TargetWindowHandle(hwnd as _)); + manage_instance_lock( + app, + InstanceLock { + mutex: hmutex as _, + window: hwnd as _, + }, + ); } Ok(()) }) - .on_event(|app, event| { - if let RunEvent::Exit = event { - destroy(app); - } - }) .build() } -pub fn destroy>(manager: &M) { - if let Some(hmutex) = manager.try_state::() { - unsafe { - ReleaseMutex(hmutex.0 as _); - CloseHandle(hmutex.0 as _); - } - } - if let Some(hwnd) = manager.try_state::() { - unsafe { DestroyWindow(hwnd.0 as _) }; - } -} - unsafe extern "system" fn single_instance_window_proc( hwnd: HWND, msg: u32, diff --git a/plugins/single-instance/tests/instance_lock.rs b/plugins/single-instance/tests/instance_lock.rs new file mode 100644 index 000000000..c07ddae2a --- /dev/null +++ b/plugins/single-instance/tests/instance_lock.rs @@ -0,0 +1,218 @@ +// Copyright 2019-2023 Tauri Programme within The Commons Conservancy +// SPDX-License-Identifier: Apache-2.0 +// SPDX-License-Identifier: MIT + +//! Runs a real primary instance in the test process and launches the test binary again as the +//! second instance, to check that it is forwarded to the primary while the lock is held and that +//! it starts normally once [`tauri_plugin_single_instance::destroy`] or +//! [`tauri::AppHandle::cleanup_before_exit`] released it. +//! +//! On Linux the lock is a D-Bus name, so these tests need a session bus and are ignored by +//! default. Run them with `dbus-run-session -- cargo test --test instance_lock -- --ignored`. + +#![cfg(not(any(target_os = "android", target_os = "ios")))] + +use std::{ + env, + process::{Child, Command, Output, Stdio}, + sync::{ + atomic::{AtomicUsize, Ordering}, + mpsc, + }, + thread, + time::{Duration, Instant}, +}; +use tauri::{ + App, Manager, RunEvent, WebviewUrl, WebviewWindowBuilder, + test::{MockRuntime, mock_builder, mock_context, noop_assets}, +}; + +const CHILD_TEST: &str = "second_instance"; +const ID_ENV: &str = "TAURI_SINGLE_INSTANCE_TEST_ID"; +// An extra test name filter that matches nothing, so it is passed through as an argument. +const MARKER: &str = "single-instance-test-marker"; +const STARTED: &str = "STARTED"; + +type Forwarded = mpsc::Receiver<(Vec, String)>; + +/// A bus name and file name friendly identifier, unique to this process and call. +fn unique_identifier() -> String { + static NEXT: AtomicUsize = AtomicUsize::new(0); + format!( + "com.tauri.SingleInstanceTest.p{}.n{}", + std::process::id(), + NEXT.fetch_add(1, Ordering::Relaxed) + ) +} + +fn build_app( + identifier: &str, + callback: Option, String)>>, +) -> App { + let mut context = mock_context(noop_assets()); + context.config_mut().identifier = identifier.into(); + let mut plugin = tauri_plugin_single_instance::Builder::new(); + if let Some(tx) = callback { + plugin = plugin.callback(move |_, args, cwd| { + let _ = tx.send((args, cwd)); + }); + } + mock_builder() + .plugin(plugin.build()) + .build(context) + .unwrap() +} + +fn primary(identifier: &str) -> (App, Forwarded) { + let (tx, rx) = mpsc::channel(); + (build_app(identifier, Some(tx)), rx) +} + +/// The Windows plugin receives the second instance's arguments through a window owned by the +/// thread that set it up, so that thread must keep pumping messages while it waits. +#[cfg(windows)] +fn pump_messages() { + use windows_sys::Win32::UI::WindowsAndMessaging::{ + DispatchMessageW, MSG, PM_REMOVE, PeekMessageW, TranslateMessage, + }; + unsafe { + let mut msg: MSG = std::mem::zeroed(); + while PeekMessageW(&mut msg, std::ptr::null_mut(), 0, 0, PM_REMOVE) != 0 { + TranslateMessage(&msg); + DispatchMessageW(&msg); + } + } +} + +#[cfg(not(windows))] +fn pump_messages() {} + +/// Launches this test binary as a second instance of the app with the given identifier. +fn launch_second_instance(identifier: &str) -> Output { + let mut child = Command::new(env::current_exe().unwrap()) + .args([ + "--include-ignored", + "--exact", + CHILD_TEST, + "--nocapture", + MARKER, + ]) + .env(ID_ENV, identifier) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let deadline = Instant::now() + Duration::from_secs(30); + loop { + if child.try_wait().unwrap().is_some() { + return child.wait_with_output().unwrap(); + } + if Instant::now() >= deadline { + kill(child); + panic!("the second instance timed out"); + } + pump_messages(); + thread::sleep(Duration::from_millis(10)); + } +} + +fn kill(mut child: Child) { + let _ = child.kill(); + let _ = child.wait(); +} + +/// Asserts the second instance handed its arguments to the primary and exited. +fn assert_forwarded(output: Output, forwarded: &Forwarded) { + assert!(output.status.success(), "{output:?}"); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + !stdout.contains(STARTED), + "the second instance started: {stdout}" + ); + let (args, cwd) = forwarded + .recv_timeout(Duration::from_secs(5)) + .expect("the primary instance did not receive the arguments"); + assert_eq!(args.last().map(String::as_str), Some(MARKER), "{args:?}"); + assert_eq!(cwd, env::current_dir().unwrap().to_string_lossy()); +} + +/// Asserts the second instance found no running instance and started normally. +fn assert_started(output: Output, forwarded: &Forwarded) { + assert!(output.status.success(), "{output:?}"); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains(STARTED), + "the second instance did not start: {stdout}" + ); + assert!(forwarded.try_recv().is_err()); +} + +/// The second instance. Only does something when launched by [`launch_second_instance`]. +#[test] +#[ignore = "launched by the other tests"] +fn second_instance() { + let Ok(identifier) = env::var(ID_ENV) else { + return; + }; + // A second instance exits inside `build` when it finds a running one. + let app = build_app(&identifier, None); + println!("{STARTED}"); + tauri_plugin_single_instance::destroy(&app); +} + +#[test] +#[cfg_attr(target_os = "linux", ignore = "needs a D-Bus session bus")] +fn destroy_releases_the_lock() { + let identifier = unique_identifier(); + let (app, forwarded) = primary(&identifier); + + assert_forwarded(launch_second_instance(&identifier), &forwarded); + assert_forwarded(launch_second_instance(&identifier), &forwarded); + + tauri_plugin_single_instance::destroy(&app); + assert_started(launch_second_instance(&identifier), &forwarded); + + // Destroying it again, and cleaning up afterwards, is a no-op. + tauri_plugin_single_instance::destroy(&app); + app.cleanup_before_exit(); +} + +#[test] +#[cfg_attr(target_os = "linux", ignore = "needs a D-Bus session bus")] +fn cleanup_before_exit_releases_the_lock() { + let identifier = unique_identifier(); + let (app, forwarded) = primary(&identifier); + + assert_forwarded(launch_second_instance(&identifier), &forwarded); + + app.cleanup_before_exit(); + assert_started(launch_second_instance(&identifier), &forwarded); + + tauri_plugin_single_instance::destroy(&app); +} + +#[test] +#[cfg_attr(target_os = "linux", ignore = "needs a D-Bus session bus")] +fn lock_is_held_during_the_exit_event() { + let identifier = unique_identifier(); + let (app, forwarded) = primary(&identifier); + WebviewWindowBuilder::new(&app, "main", WebviewUrl::default()) + .build() + .unwrap(); + + let (exit_tx, exit_rx) = mpsc::channel(); + let id = identifier.clone(); + app.run(move |app, event| match event { + // The mock runtime exits once its last window is gone. + RunEvent::Ready => app.get_webview_window("main").unwrap().destroy().unwrap(), + RunEvent::Exit => { + exit_tx.send(launch_second_instance(&id)).unwrap(); + } + _ => {} + }); + + let during_exit = exit_rx.try_recv().expect("RunEvent::Exit was not emitted"); + assert_forwarded(during_exit, &forwarded); + // `App::run` cleans up after the exit event. + assert_started(launch_second_instance(&identifier), &forwarded); +}