Conversation
|
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. Maybe it is still unsound that |
Package Changes Through db99474There are 9 changes which include tauri with minor, @tauri-apps/cli with minor, tauri-cli with minor, tauri-utils with patch, tauri-build with patch, tauri-macos-sign with patch, tauri-bundler with minor, tauri-runtime-wry with minor, tauri-runtime with minor Planned Package VersionsThe following package releases are the planned based on the context of changes in this pull request.
Add another change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
|
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 std::thread::spawn(move || match new_window_req_handler(uri, features) { ... }); |
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:
fn on_eventfrom thePlugintrait are not SyncWindowsStoreis sandwiched between two types that are both not Sync, it contains types that are not Sync and it itself is contained inand
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
WindowsStoresSyncor make them both not Sync. Attempting to make both of these structsSyncis 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 bothWindowsStoreandDispatcherMainThreadContextare mistaken.Removing the Sync impl of
DispatcherMainThreadContextleads to another rat's nest of errors.Contextused to inherit Sync based on its fields, so removing Sync fromDispatcherMainThreadContextmakes it not Sync as well.I can't reason whether
Contextshould 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.