Skip to content

Fix unsound Send+Sync impl#14797

Closed
sftse wants to merge 3 commits into
tauri-apps:devfrom
sftse:fix-unsound
Closed

Fix unsound Send+Sync impl#14797
sftse wants to merge 3 commits into
tauri-apps:devfrom
sftse:fix-unsound

Conversation

@sftse
Copy link
Copy Markdown
Contributor

@sftse sftse commented Jan 19, 2026

This is a tiny breaking change to fix an unsoundness in the public API of tauri.
There is no non-breaking change that could fix this, and anybody without a Sync+Send error type is doing something shady anyway.

The issue is we can have an error type with an Rc that is then laundered by the Tauri unsafe impl into a SetupError which can be freely sent to a different thread. See this modification of helloworld.rs

fn main() {
  use std::cell::RefCell;
  use std::rc::Rc;

  thread_local! {
      static LOCAL: RefCell<Option<Rc<()>>> = RefCell::new(None);
  };
  LOCAL.with(|m| *m.borrow_mut() = Some(Rc::new(())));
  // Rc -> SetupError(Box<dyn std::error::Error + Send + Sync>) Laundromat
  // free to send one end to another thread
  if let tauri::Error::Setup(e) = tauri::Builder::default()
    .setup(move |_| Err(Error(LOCAL.with(|m| m.borrow_mut().take().unwrap())).into()))
    .invoke_handler(tauri::generate_handler![])
    .run(tauri::generate_context!(
      "../../examples/helloworld/tauri.conf.json"
    ))
    .unwrap_err()
  {
    std::thread::spawn(move || e);
  };
}

#[derive(Clone)]
struct Error(std::rc::Rc<()>);

impl std::fmt::Debug for Error {
  fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
    todo!()
  }
}

impl std::fmt::Display for Error {
  fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
    todo!()
  }
}

impl std::error::Error for Error {}

There is another soundness issue in tauri-runtime-wry's WindowsStore where there's an unsafe Sync impl even though the inner type is public. I haven't been able to verify whether the code was ever sound, but since the inner field was made public it definitely became unsound, RefCell is not Sync, but the wrapper type is, so we can trivially send references to WindowsStore to multiple threads and then open them up to references to RefCell.
The safety comments "// SAFETY: we ensure this type is only used on the main thread." are incorrect.

@sftse sftse requested a review from a team as a code owner January 19, 2026 17:28
@github-actions
Copy link
Copy Markdown
Contributor

Package Changes Through 1e8fca7

There are 9 changes which include tauri-utils with patch, tauri-build with patch, tauri-cli with minor, tauri-macos-sign with patch, @tauri-apps/cli with minor, tauri with minor, tauri-bundler with minor, tauri-runtime-wry with minor, tauri-runtime 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-utils 2.8.1 2.8.2
tauri-macos-sign 2.3.2 2.3.3
tauri-bundler 2.7.5 2.8.0
tauri-runtime 2.9.2 2.10.0
tauri-runtime-wry 2.9.3 2.10.0
tauri-codegen 2.5.2 2.5.3
tauri-macros 2.5.2 2.5.3
tauri-plugin 2.5.2 2.5.3
tauri-build 2.5.3 2.5.4
tauri 2.9.5 2.10.0
@tauri-apps/cli 2.9.6 2.10.0
tauri-cli 2.9.6 2.10.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

@FabianLars
Copy link
Copy Markdown
Member

@WSH032 if you're still around, will this cause issues for you?

@sftse
Copy link
Copy Markdown
Contributor Author

sftse commented Jan 19, 2026

While on the topic of unsoundness, I previously filed #13257 and wry PR1571 to tighten up the code and be a bit more defensive about unsafe in general, but it seems they never got the necessary traction. Would appreciate some feedback regarding how to proceed with them.

@sftse
Copy link
Copy Markdown
Contributor Author

sftse commented Jan 20, 2026

Woops, I should have run the code instead of just compiling it, my bad. The setup error is caught here and stringified. So luckily there isn't a way to get an unsound crash from it. I'll open a separate issue for the remaining issue.

        if let Err(e) = setup(&mut self) {
          panic!("Failed to setup app: {e}");
        }

@sftse sftse closed this Jan 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants