refactor: move unsafe send sync impl up to Context

[sftse]

Fixes #14801

Depends on wry PR 1657.

I'm not intimately familiar with what is going on in this code, so I'm using heuristics to guide my fix:

• most of the arguments passed to fn on_event from the Plugin trait are not Sync
• WindowsStore is sandwiched between two types that are both not Sync, it contains types that are not Sync and it itself is contained in

unsafe impl<T: UserEvent> Sync for DispatcherMainThreadContext<T> {}

pub struct DispatcherMainThreadContext<T: UserEvent> {
  pub window_target: EventLoopWindowTarget<Message<T>>,
  pub web_context: WebContextStore,
  // changing this to an Rc will cause frequent app crashes.
  pub windows: Arc<WindowsStore>,
  #[cfg(feature = "tracing")]
  pub active_tracing_spans: ActiveTraceSpanStore,
}

and

pub struct EventLoopIterationContext<'a, T: UserEvent> {
  pub callback: &'a mut (dyn FnMut(RunEvent<T>) + 'static),
  pub window_id_map: WindowIdStore,
  pub windows: Arc<WindowsStore>,
  #[cfg(feature = "tracing")]
  pub active_tracing_spans: ActiveTraceSpanStore,
}

of which the last is not Sync.

For basic soundness based on the examples in #14801 if a non-Sync field is public in a public struct that struct must be not Sync as well.
We have two choices, make both these structs that contain public WindowsStores Sync or make them both not Sync. Attempting to make both of these structs Sync is a rat's nest, so let's take the alternative, let's assume they must be both not Sync. This implies the Sync impl of both WindowsStore and DispatcherMainThreadContext are mistaken.

Removing the Sync impl of DispatcherMainThreadContext leads to another rat's nest of errors. Context used to inherit Sync based on its fields, so removing Sync from DispatcherMainThreadContext makes it not Sync as well.

I can't reason whether Context should or should not be Sync, but in any case we are overriding Sync "at the edges" of the code, so there is a larger space in which the absence of Sync will be caught by the compiler.

In summary, these changes strictly reduce the ways to be unsound. More cases are caught that were previously not sound.

[sftse]

Because of

unsafe impl<T: UserEvent> Send for DispatcherMainThreadContext<T> {}

#[derive(Clone)]
pub struct DispatcherMainThreadContext<T: UserEvent> {
  pub window_target: EventLoopWindowTarget<Message<T>>,
  pub web_context: WebContextStore,
  // changing this to an Rc will cause frequent app crashes.
  pub windows: Arc<WindowsStore>,
  #[cfg(feature = "tracing")]
  pub active_tracing_spans: ActiveTraceSpanStore,
}

there's another change necessary, originally missed.
Arc<T> is only Send if T: Send, but the Send on DispatcherMainThreadContext allows an unsound bypass:
Clone DispatcherMainThreadContext -> send clone to another thread -> take inner Arc<WindowsStore> -> access WindowsStore unsynchronized from both threads. So we have to do the same fix as with Sync and remove the Send impl, delay it until Context.

Maybe it is still unsound that Context is Send and Sync, but the scope for unsound mistakes is reduced.

[github-actions]

Package Changes Through 6c29e7b

There are 2 changes which include tauri-runtime-wry with minor, tauri-bundler with patch

Planned Package Versions

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

package	current	next
tauri-bundler	2.9.2	2.9.3
tauri-runtime-wry	2.11.2	2.12.0
tauri	2.11.2	2.11.3
@tauri-apps/cli	2.11.2	2.11.3
tauri-cli	2.11.2	2.11.3

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

---

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

[sftse]

As the broken CI on wry PR 1657 shows, that fix was a bit optimistic.

I don't know when I can further look into this, but I've pushed a fix that pulls the unsafety at least to the internals of Tauri instead of exposing it publically. Maybe this is enough of an improvement to be merged?

Summarizing, tauri currently exposes unsoundness in its public api, we might be interested in merging something that breaks as little as possible to fix this, this is such a minimal suggestion.

The fact that wry does end up needing Send here for the request handler is a bit rough though. Somebody with a better grasp of the code should probably be looking into this.

          std::thread::spawn(move || match new_window_req_handler(uri, features) { ... });

[Legend-Master]

The way this moved the unsafe Send and Sync impl to Context makes sense to me

[Legend-Master]

I think maybe we can do this the thing I did in 021476b

[sftse]

Can we elaborate on this in another PR? I don't have the bandwidth except to see this merged.

[sftse]

This would probably fix #10001

[Legend-Master]

#10001 is different that it's a panic on global_shortcut_manager that no longer exists in v2

[sftse]

Might make sense to close it?

[Legend-Master]

Since this is breaking change in the tauri-runtime-wry crate, I will wait a bit to merge this in tauri 2.12 window to publish it as a minor bump