Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(core): Replace Rc with Arc to prevent crashes when sending events #8402

Merged
merged 5 commits into from
Dec 20, 2023

Conversation

FabianLars
Copy link
Member

I feel like this is more of a workaround than a fix but this reduced the crashes on macos for me. That said, the issue i was trying to fix was on windows but when i worked on this commit i couldn't repro it with and without this change. The issue seems to be influenced by available system resources or something because it's not the first time i wasn't able to reproduce it.

This may close #8177 but i'd like @i-c-b and/or @JJeris to try it out (since they were able to reproduce it more reliably than me).

p.s. i don't think i'm capable enough of doing so but we should still find out what's actually happening here or alternatively change the types so that clippy doesn't complain about Arc when it's actually needed (if possible).

@FabianLars FabianLars requested a review from a team as a code owner December 14, 2023 17:34
@JJeris
Copy link

JJeris commented Dec 15, 2023

What was this workaround? The iterator or the new thing discovered by the other user?
Ill test it out today, if i can.

@i-c-b
Copy link
Contributor

i-c-b commented Dec 15, 2023

This seems to fix the issue for Linux. On Windows, it seems that emitting more than 12725 events at time causes emit to fail with runtime error: failed to send message to the webview; unsure what's going on there.

event-flooder crashes with v1.5.2, v1.5.3, and the bleeding-edge 1.x branch which includes the clipboard changes. All had nearly instant crashes when I tried 1,000,000 events. The moment I switched to the event-crash branch, the crashing stopped and I could no longer reproduce it no matter how many events I emitted.

What was this workaround? The iterator or the new thing discovered by the other user?

In Tauri v1.5.0, the event system switched from std::sync::Arc to std::rc::Rc. This workaround reverts that change as a (hopefully) temporary measure while a more permanent solution is found. The main difference is that Arc (Atomically Reference Counted) is thread-safe and Rc (Reference Counted) isn't.

@lucasfernog
Copy link
Member

Windows issue is Tried to wake up a closed EventLoop. I'm investigating.

@lucasfernog lucasfernog merged commit b2f83f0 into 1.x Dec 20, 2023
23 checks passed
@lucasfernog lucasfernog deleted the event-crash branch December 20, 2023 15:13
lucasfernog added a commit that referenced this pull request Aug 13, 2024
Rc is not thread safe, so even though we are only using these types on the main thread, we need to ensure clones are safe across threads.

This is a follow up for #8402
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants