Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix(macOS): fix race condition in webview handler #1425

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

linw1995
Copy link

@linw1995 linw1995 requested a review from a team as a code owner November 23, 2024 15:21
Copy link
Contributor

Package Changes Through 48e31fa

There are 1 changes which include wry 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
wry 0.47.2 0.48.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
Member

Does this really help? We already check this 2 instructions above in check_webview_id_valid 🤔

@linw1995
Copy link
Author

Does this really help? We already check this 2 instructions above in check_webview_id_valid 🤔

work like a charm.

  14:        0x103430874 - core::panicking::panic_fmt::ha4b80a05b9fff47a
                               at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/core/src/panicking.rs:74:14
  15:        0x102fa8054 - objc2::__macro_helpers::declared_ivars::get_initialized_ivar_ptr::h92376258763b2729
                               at /Users/linw1995/.cargo/registry/src/index.crates.io-6f17d22bba15001f/objc2-0.5.2/src/__macro_helpers/declared_ivars.rs:403:17
  16:        0x102f9dc44 - objc2::top_level_traits::DeclaredClass::ivars_mut::h2f3cb44ff3ce2dab
                               at /Users/linw1995/.cargo/registry/src/index.crates.io-6f17d22bba15001f/objc2-0.5.2/src/top_level_traits.rs:355:34
  17:        0x102fa0474 - wry::wkwebview::class::wry_web_view::WryWebView::remove_custom_task_key::hd2a6c5fea7c15784
                               at /Users/linw1995/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wry-0.47.1/src/wkwebview/class/wry_web_view.rs:149:5

Simply performing checks is not enough; it needs to be locked to prevent race conditions.

@pewsheen
Copy link
Contributor

pewsheen commented Nov 28, 2024

I think this will mitigate the problem, however it could still happen if the webview is closed just before calling remove_custom_task_key.

MacOS expects us to capture the NSExecption to tell if the task is dead, but that's impossible for now. The async command will be unreliable on MacOS until Objc2 can bubble up an NSException instead of panicking so that we can remove those weird checks.

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.

3 participants