fix: ensure RunEvent::Exit is triggered on restart #12313

anatawa12 · 2025-01-08T06:20:14Z

Fixes #12310
Fixes #11392
Fixes tauri-apps/plugins-workspace#1692
Fixes tauri-apps/plugins-workspace#2256

…xit` event before exiting the application.

github-actions · 2025-01-08T06:42:09Z

Package Changes Through `1c58985`

There are 12 changes which include tauri with minor, tauri-runtime with minor, tauri-runtime-wry with minor, tauri-utils with minor, tauri-cli with minor, @tauri-apps/cli with minor, @tauri-apps/api with minor, tauri-bundler with patch, tauri-build with minor, tauri-codegen with minor, tauri-macros with minor, tauri-plugin with minor

Planned Package Versions

The following package releases are the planned based on the context of changes in this pull request.

package	current	next
@tauri-apps/api	2.3.0	2.4.0
tauri-utils	2.2.0	2.3.0
tauri-bundler	2.2.4	2.2.5
tauri-runtime	2.4.0	2.5.0
tauri-runtime-wry	2.4.1	2.5.0
tauri-codegen	2.0.5	2.1.0
tauri-macros	2.0.5	2.1.0
tauri-plugin	2.0.5	2.1.0
tauri-build	2.0.6	2.1.0
tauri	2.3.1	2.4.0
@tauri-apps/cli	2.3.1	2.4.0
tauri-cli	2.3.1	2.4.0

Add another change file through the GitHub UI by following this link.

Read about change files or the docs at github.com/jbolda/covector

anatawa12 · 2025-01-08T06:46:58Z

I've fixed format error. sorry

SergeFan · 2025-02-12T14:31:06Z

Any updates to this PR?

anatawa12 · 2025-02-12T15:33:12Z

I'm waiting for review.

anatawa12 · 2025-02-12T15:37:53Z

The CI failure is corepack issue (nodejs/corepack#625) so it's not my fault and nothing I can do.

Legend-Master · 2025-02-18T12:23:01Z

Maybe we can just store a bool in AppManager to indicate that we need to restart and we can restart when we receive the next RuntimeRunEvent::Exit event in App::run?

Edit: actually, no since we have marked it as diverging function, maybe we can do #10171 (comment), by introducing request_exit and request_restart? Or maybe delay and break it in v3

anatawa12 · 2025-02-22T18:04:31Z

Yes I think we can add request_restart (and possibly force_request_restart which will not possible to cancel exit with prevent_exit) and deprecate current one (and possibly remove in v3)
With new approach, we have to fix every use cases of restart in each plugins.

I may make separated PR for this new approach.

Legend-Master · 2025-02-23T01:11:33Z

With new approach, we have to fix every use cases of restart in each plugins.

It should be fine, I think only the process plugin is affected in all the official plugins? (the restart command)

anatawa12 · 2025-02-24T19:03:09Z

Iseached for use cases and restart command in process plugin and all their use cases (even in js) would also needs migration.

lucasfernog · 2025-02-28T12:03:05Z

this is now becoming a really complicated solution 😂 I had to push a change to make it not deadlock when restart() is called on the main thread

lucasfernog · 2025-02-28T12:07:51Z

crates/tauri/src/manager/mod.rs

+
+  // the value is set to true when the event loop is already exited
+  event_loop_exit_mutex: Mutex<bool>,
+  event_loop_exit_condvar: std::sync::Condvar,


i'm not super familiar with condvar, is there an advantage in using this over e.g. a multi producer multi consumer channel?

AFAIK, mcmc channel can only one thread at once but condvar can wake multiple threads at once so I used condvar

anatawa12 · 2025-02-28T12:20:14Z

I don't enough time to implement request_restart so I'm keeping this PR as-is but should we push forward this complex solution instead of reimplement with request_restart?

lucasfernog · 2025-02-28T12:30:48Z

I'm considering going forward with both - fix the restart() behavior but also deprecate it and implement request_restart

Legend-Master · 2025-02-28T13:21:13Z

I'm thinking if something like this would just work for request_restart

diff --git a/crates/tauri/src/app.rs b/crates/tauri/src/app.rs
index 728183d69..7813097ca 100644
--- a/crates/tauri/src/app.rs
+++ b/crates/tauri/src/app.rs
@@ -10,6 +10,7 @@ use crate::{
   },
   manager::{webview::UriSchemeProtocol, AppManager, Asset},
   plugin::{Plugin, PluginStore},
+  process::restart,
   resources::ResourceTable,
   runtime::{
     window::{WebviewEvent as RuntimeWebviewEvent, WindowEvent as RuntimeWindowEvent},
@@ -42,7 +43,7 @@ use std::{
   borrow::Cow,
   collections::HashMap,
   fmt,
-  sync::{mpsc::Sender, Arc, MutexGuard},
+  sync::{atomic, mpsc::Sender, Arc, MutexGuard},
 };
 
 use crate::{event::EventId, runtime::RuntimeHandle, Event, EventTarget};
@@ -477,6 +478,18 @@ impl<R: Runtime> AppHandle<R> {
     crate::process::restart(&self.env());
   }
 
+  /// Restarts the app by triggering [`RunEvent::ExitRequested`] with code [`RESTART_EXIT_CODE`] and [`RunEvent::Exit`]..
+  pub fn request_restart(&self) {
+    self
+      .manager
+      .restart_on_exit
+      .store(true, atomic::Ordering::Relaxed);
+    if self.runtime_handle.request_exit(RESTART_EXIT_CODE).is_err() {
+      self.cleanup_before_exit();
+      crate::process::restart(&self.env());
+    }
+  }
+
   /// Sets the activation policy for the application. It is set to `NSApplicationActivationPolicyRegular` by default.
   ///
   /// # Examples
@@ -1084,6 +1097,9 @@ impl<R: Runtime> App<R> {
         let event = on_event_loop_event(&app_handle, RuntimeRunEvent::Exit, &manager);
         callback(&app_handle, event);
         app_handle.cleanup_before_exit();
+        if self.manager.restart_on_exit.load(atomic::Ordering::Relaxed) {
+          restart(&self.env());
+        }
       }
       _ => {
         let event = on_event_loop_event(&app_handle, event, &manager);
diff --git a/crates/tauri/src/manager/mod.rs b/crates/tauri/src/manager/mod.rs
index 882b141ca..ec6610864 100644
--- a/crates/tauri/src/manager/mod.rs
+++ b/crates/tauri/src/manager/mod.rs
@@ -6,7 +6,7 @@ use std::{
   borrow::Cow,
   collections::HashMap,
   fmt,
-  sync::{Arc, Mutex, MutexGuard},
+  sync::{atomic::AtomicBool, Arc, Mutex, MutexGuard},
 };
 
 use serde::Serialize;
@@ -221,6 +221,8 @@ pub struct AppManager<R: Runtime> {
   pub(crate) invoke_key: String,
 
   pub(crate) channel_interceptor: Option<ChannelInterceptor<R>>,
+
+  pub(crate) restart_on_exit: AtomicBool,
 }
 
 impl<R: Runtime> fmt::Debug for AppManager<R> {
@@ -322,6 +324,7 @@ impl<R: Runtime> AppManager<R> {
       resources_table: Arc::default(),
       invoke_key,
       channel_interceptor,
+      restart_on_exit: AtomicBool::new(false),
     }
   }

lucasfernog · 2025-02-28T16:04:32Z

I'm thinking if something like this would just work for request_restart

diff --git a/crates/tauri/src/app.rs b/crates/tauri/src/app.rs
index 728183d69..7813097ca 100644
--- a/crates/tauri/src/app.rs
+++ b/crates/tauri/src/app.rs
@@ -10,6 +10,7 @@ use crate::{
   },
   manager::{webview::UriSchemeProtocol, AppManager, Asset},
   plugin::{Plugin, PluginStore},
+  process::restart,
   resources::ResourceTable,
   runtime::{
     window::{WebviewEvent as RuntimeWebviewEvent, WindowEvent as RuntimeWindowEvent},
@@ -42,7 +43,7 @@ use std::{
   borrow::Cow,
   collections::HashMap,
   fmt,
-  sync::{mpsc::Sender, Arc, MutexGuard},
+  sync::{atomic, mpsc::Sender, Arc, MutexGuard},
 };
 
 use crate::{event::EventId, runtime::RuntimeHandle, Event, EventTarget};
@@ -477,6 +478,18 @@ impl<R: Runtime> AppHandle<R> {
     crate::process::restart(&self.env());
   }
 
+  /// Restarts the app by triggering [`RunEvent::ExitRequested`] with code [`RESTART_EXIT_CODE`] and [`RunEvent::Exit`]..
+  pub fn request_restart(&self) {
+    self
+      .manager
+      .restart_on_exit
+      .store(true, atomic::Ordering::Relaxed);
+    if self.runtime_handle.request_exit(RESTART_EXIT_CODE).is_err() {
+      self.cleanup_before_exit();
+      crate::process::restart(&self.env());
+    }
+  }
+
   /// Sets the activation policy for the application. It is set to `NSApplicationActivationPolicyRegular` by default.
   ///
   /// # Examples
@@ -1084,6 +1097,9 @@ impl<R: Runtime> App<R> {
         let event = on_event_loop_event(&app_handle, RuntimeRunEvent::Exit, &manager);
         callback(&app_handle, event);
         app_handle.cleanup_before_exit();
+        if self.manager.restart_on_exit.load(atomic::Ordering::Relaxed) {
+          restart(&self.env());
+        }
       }
       _ => {
         let event = on_event_loop_event(&app_handle, event, &manager);
diff --git a/crates/tauri/src/manager/mod.rs b/crates/tauri/src/manager/mod.rs
index 882b141ca..ec6610864 100644
--- a/crates/tauri/src/manager/mod.rs
+++ b/crates/tauri/src/manager/mod.rs
@@ -6,7 +6,7 @@ use std::{
   borrow::Cow,
   collections::HashMap,
   fmt,
-  sync::{Arc, Mutex, MutexGuard},
+  sync::{atomic::AtomicBool, Arc, Mutex, MutexGuard},
 };
 
 use serde::Serialize;
@@ -221,6 +221,8 @@ pub struct AppManager<R: Runtime> {
   pub(crate) invoke_key: String,
 
   pub(crate) channel_interceptor: Option<ChannelInterceptor<R>>,
+
+  pub(crate) restart_on_exit: AtomicBool,
 }
 
 impl<R: Runtime> fmt::Debug for AppManager<R> {
@@ -322,6 +324,7 @@ impl<R: Runtime> AppManager<R> {
       resources_table: Arc::default(),
       invoke_key,
       channel_interceptor,
+      restart_on_exit: AtomicBool::new(false),
     }
   }

that should work - i'm fixing some other bugs but i can test this later

leads to a deadlock currently

.changes/restart-may-not-publish-exit-event.md

Legend-Master · 2025-03-01T07:15:08Z

@lucasfernog the deadlock around the event loop seems to be a recurring theme now, could you tell where the deadlock happens? I only know the plugin lock which can be solved by not having the callbacks being MutFn so we don't need a lock to call them (breaking change so needs to wait for v3)

Also I think this main thread exit implementation should probably be moved to tauri-runtime-wry? Using UnsafeCell here feels so wrong to me

lucasfernog · 2025-03-01T11:05:41Z

In this case we have a deadlock because we're running wait_for_event_loop_exit in the main thread, meaning notify_event_loop_exit does not have a chance to be triggered. The problem is that request_exit uses the event loop proxy, so the Exit callback is only triggered in the next iteration

tauri/crates/tauri-runtime-wry/src/lib.rs

Line 2377 in 3fb8d7c

    
           // NOTE: request_exit cannot use the `send_user_message` function because it accesses the event loop callback

I'll see if there's a way to rewrite it in a better way, but I'm not super confident.

lucasfernog · 2025-03-01T11:06:37Z

I would probably need to use the same mechanism to store the event loop callback and use it in a thread-safe struct (but ensuring it only gets called in the main thread)

Legend-Master · 2025-03-01T11:28:29Z

In this case we have a deadlock because we're running wait_for_event_loop_exit in the main thread, meaning notify_event_loop_exit does not have a chance to be triggered. The problem is that request_exit uses the event loop proxy, so the Exit callback is only triggered in the next iteration

Got it now, so the main problem is that the function is blocking a the event loop while waiting for the event loop to move and receive the next message

// NOTE: request_exit cannot use the send_user_message function because it accesses the event loop callback

I wonder why is this though, if we're not blocking on the main thread, this should be fine?

lucasfernog · 2025-03-01T23:32:04Z

In this case we have a deadlock because we're running wait_for_event_loop_exit in the main thread, meaning notify_event_loop_exit does not have a chance to be triggered. The problem is that request_exit uses the event loop proxy, so the Exit callback is only triggered in the next iteration

Got it now, so the main problem is that the function is blocking a the event loop while waiting for the event loop to move and receive the next message

// NOTE: request_exit cannot use the send_user_message function because it accesses the event loop callback

I wonder why is this though, if we're not blocking on the main thread, this should be fine?

yes you're right
"it should be fine" yes, but that's why we're using UnsafeCell, we're calling from possibly another thread but checking if we're on the main one so we do not deadlock

Legend-Master · 2025-03-02T03:56:08Z

"it should be fine" yes

I see, so maybe we should update the comments to include where it would dead lock if we do it on the main thread? I was quite confused by it (still somewhat confused right now 😂)

And just one more thing, since the callback is a MutFn, if we call restart inside that callback, wouldn't we break the only a single mutable reference at a time through UnsafeCell?

lucasfernog · 2025-03-02T10:46:44Z

"it should be fine" yes

I see, so maybe we should update the comments to include where it would dead lock if we do it on the main thread? I was quite confused by it (still somewhat confused right now 😂)

And just one more thing, since the callback is a MutFn, if we call restart inside that callback, wouldn't we break the only a single mutable reference at a time through UnsafeCell?

yeah that's something i was thinking about too.. maybe we should crash instead (so it's neither a deadlock nor a crazy unsafecell usage)
EDIT: or maybe just not trigger RunEvent::Exit in this case

Legend-Master · 2025-03-02T11:55:33Z

In this case I'm leaning towards just leave this function as is and deprecate it, then introduce request_restart to be honest

Since this function has never triggered the Exit event before, I think it's fine to just change the docs to say it doesn't trigger the event and restart immediately, if you want the event, use request_restart instead

lucasfernog · 2025-03-02T12:59:59Z

In this case I'm leaning towards just leave this function as is and deprecate it, then introduce request_restart to be honest

Since this function has never triggered the Exit event before, I think it's fine to just change the docs to say it doesn't trigger the event and restart immediately, if you want the event, use request_restart instead

but that's not the only problem this PR tries to solve.. it also fixes the race condition for restart/exit

Legend-Master · 2025-03-02T13:11:47Z

but that's not the only problem this PR tries to solve.. it also fixes the race condition for restart/exit

Wait, what race condition?

lucasfernog · 2025-03-15T16:49:13Z

but that's not the only problem this PR tries to solve.. it also fixes the race condition for restart/exit

Wait, what race condition?

@Legend-Master #11392 (comment)

Legend-Master · 2025-03-16T00:22:20Z

but that's not the only problem this PR tries to solve.. it also fixes the race condition for restart/exit

Wait, what race condition?

@Legend-Master #11392 (comment)

I see, but I still feel like we could just remove self.runtime_handle.request_exit(RESTART_EXIT_CODE) and call crate::process::restart(&self.env()) directly, anyways, I'm not opposed to this either

anatawa12 added 2 commits January 8, 2025 15:01

fix: AppHandle::restart() may not send RunEvent::Exit event

f5bad2a

docs: add changelog: AppHandle::restart() may not send `RunEvent::E…

65e44cc

…xit` event before exiting the application.

anatawa12 requested a review from a team as a code owner January 8, 2025 06:20

style: cargo fmt

0a7b31e

Merge branch 'dev' into restart-wait-for-exit

c8b8ce5

Merge remote-tracking branch 'origin/dev' into restart-wait-for-exit

af24c1f

lucasfernog reviewed Feb 28, 2025

View reviewed changes

avoid deadlock on main thread

055ffd7

lucasfernog force-pushed the restart-wait-for-exit branch from 0b9faf6 to 055ffd7 Compare February 28, 2025 12:14

do not let the restart be prevented

204486f

leads to a deadlock currently

lucasfernog reviewed Feb 28, 2025

View reviewed changes

.changes/restart-may-not-publish-exit-event.md Outdated Show resolved Hide resolved

Apply suggestions from code review

ba4f6e3

lucasfernog changed the title ~~fix: AppHandle::restart() may not send RunEvent::Exit event~~ fix: ensure RunEvent::Exit is triggered on restart Feb 28, 2025

github-project-automation bot added this to Roadmap Feb 28, 2025

github-project-automation bot moved this to 📬Proposal in Roadmap Feb 28, 2025

lint

b5ab76c

lucasfernog previously approved these changes Feb 28, 2025

View reviewed changes

lucasfernog added 2 commits March 2, 2025 08:04

do not emit RunEvent on main thread

a27001c

re-export RESTART_EXIT_CODE

112db45

lucasfernog dismissed their stale review via 112db45 March 2, 2025 11:49

Legend-Master mentioned this pull request Mar 3, 2025

feat: introduce request_restart #12874

Merged

anatawa12 mentioned this pull request Mar 3, 2025

[bug?] .deb package build with tauri does not include version requirements for libc6 and libstdc++6 #12883

Open

Merge remote-tracking branch 'origin/dev' into restart-wait-for-exit

1c58985

lucasfernog approved these changes Mar 15, 2025

View reviewed changes

lucasfernog merged commit b05f82d into tauri-apps:dev Mar 15, 2025
20 checks passed

github-project-automation bot moved this from 📬Proposal to 🔎 In audit in Roadmap Mar 15, 2025

github-actions bot mentioned this pull request Mar 16, 2025

Apply Version Updates From Current Changes #12856

Merged

lucasfernog mentioned this pull request Apr 13, 2025

fix(windows): The restart api does not work in single-instance mode #11684

Closed

Uh oh!

fix: ensure RunEvent::Exit is triggered on restart #12313

fix: ensure RunEvent::Exit is triggered on restart #12313

Uh oh!

Conversation

anatawa12 commented Jan 8, 2025

Uh oh!

github-actions bot commented Jan 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Package Changes Through 1c58985

Uh oh!

anatawa12 commented Jan 8, 2025

Uh oh!

SergeFan commented Feb 12, 2025

Uh oh!

anatawa12 commented Feb 12, 2025

Uh oh!

anatawa12 commented Feb 12, 2025

Uh oh!

Legend-Master commented Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anatawa12 commented Feb 22, 2025

Uh oh!

Legend-Master commented Feb 23, 2025

Uh oh!

anatawa12 commented Feb 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lucasfernog commented Feb 28, 2025

Uh oh!

lucasfernog Feb 28, 2025

Choose a reason for hiding this comment

Uh oh!

anatawa12 Feb 28, 2025

Choose a reason for hiding this comment

Uh oh!

anatawa12 commented Feb 28, 2025

Uh oh!

lucasfernog commented Feb 28, 2025

Uh oh!

Legend-Master commented Feb 28, 2025

Uh oh!

lucasfernog commented Feb 28, 2025

Uh oh!

Uh oh!

Legend-Master commented Mar 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lucasfernog commented Mar 1, 2025

Uh oh!

lucasfernog commented Mar 1, 2025

Uh oh!

Legend-Master commented Mar 1, 2025

Uh oh!

lucasfernog commented Mar 1, 2025

Uh oh!

Legend-Master commented Mar 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lucasfernog commented Mar 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Legend-Master commented Mar 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lucasfernog commented Mar 2, 2025

Uh oh!

Legend-Master commented Mar 2, 2025

Uh oh!

lucasfernog commented Mar 15, 2025

Uh oh!

Uh oh!

Legend-Master commented Mar 16, 2025

Uh oh!

Uh oh!

github-actions bot commented Jan 8, 2025 •

edited

Loading

Package Changes Through `1c58985`

Legend-Master commented Feb 18, 2025 •

edited

Loading

anatawa12 commented Feb 24, 2025 •

edited

Loading

Legend-Master commented Mar 1, 2025 •

edited

Loading

Legend-Master commented Mar 2, 2025 •

edited

Loading

lucasfernog commented Mar 2, 2025 •

edited

Loading

Legend-Master commented Mar 2, 2025 •

edited

Loading