fix(single-instance): release the lock on cleanup_before_exit instead of RunEvent::Exit (#3482)

* add configurable destroy_on_exit to single instance plugin

* add change file

* document destroy_on_exit

* move the destroy to rely on the Resource close

* add integration test

* fix android/ios

---------

Co-authored-by: Lucas Nogueira <lucas@tauri.app>

.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.

.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::"

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 }
 

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<R: Runtime, F: FnMut(&AppHandle<R>, Vec<String>, 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<R: Runtime, M: Manager<R>>(manager: &M) {
-    platform_impl::destroy(manager)
+    if let Some(lock) = manager.try_state::<InstanceLockId>() {
+        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<R: Runtime, M: Manager<R>>(manager: &M, lock: impl Resource) {
+    let rid = manager.resources_table().add(lock);
+    manager.manage(InstanceLockId(rid));
 }
 
 /// Builds the single-instance plugin.

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<R: Runtime> {
     callback: Box<SingleInstanceCallback<R>>,
@@ -26,8 +40,6 @@ impl<R: Runtime> SingleInstanceDBus<R> {
     }
 }
 
-struct DBusName(String);
-
 pub fn init<R: Runtime>(
     callback: Box<SingleInstanceCallback<R>>,
     dbus_id: Option<String>,
@@ -71,7 +83,13 @@ pub fn init<R: Runtime>(
                 .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<R: Runtime>(
                 }
             }
 
-            app.manage(DBusName(dbus_name));
-
             Ok(())
         })
-        .on_event(move |app, event| {
-            if let RunEvent::Exit = event {
-                destroy(app);
-            }
-        })
         .build()
 }
-
-pub fn destroy<R: Runtime, M: Manager<R>>(manager: &M) {
-    if let Some(connection) = manager.try_state::<ConnectionHandle>()
-        && let Some(dbus_name) = manager
-            .try_state::<DBusName>()
-            .and_then(|name| WellKnownName::try_from(name.0.clone()).ok())
-    {
-        let _ = connection.0.release_name(dbus_name);
-    }
-}

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<R: Runtime>(cb: Box<SingleInstanceCallback<R>>) -> TauriPlugin<R> {
     plugin::Builder::new("single-instance")
         .setup(|app, _api| {
@@ -32,6 +43,7 @@ pub fn init<R: Runtime>(cb: Box<SingleInstanceCallback<R>>) -> TauriPlugin<R> {
                         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<R: Runtime>(cb: Box<SingleInstanceCallback<R>>) -> TauriPlugin<R> {
             }
             Ok(())
         })
-        .on_event(|app, event| {
-            if let RunEvent::Exit = event {
-                destroy(app);
-            }
-        })
         .build()
 }
 
-pub fn destroy<R: Runtime, M: Manager<R>>(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(), "_");
 

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<R: Runtime> {
     app: AppHandle<R>,
@@ -145,39 +160,26 @@ pub fn init<R: Runtime>(callback: Box<SingleInstanceCallback<R>>) -> 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::<R>(&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<R: Runtime, M: Manager<R>>(manager: &M) {
-    if let Some(hmutex) = manager.try_state::<MutexHandle>() {
-        unsafe {
-            ReleaseMutex(hmutex.0 as _);
-            CloseHandle(hmutex.0 as _);
-        }
-    }
-    if let Some(hwnd) = manager.try_state::<TargetWindowHandle>() {
-        unsafe { DestroyWindow(hwnd.0 as _) };
-    }
-}
-
 unsafe extern "system" fn single_instance_window_proc<R: Runtime>(
     hwnd: HWND,
     msg: u32,

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>, 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<mpsc::Sender<(Vec<String>, String)>>,
+) -> App<MockRuntime> {
+    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<MockRuntime>, 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);
+}