gpui: Fix `update_window` to `borrow_mut` will crash on Windows
Release Notes:
- N/A
When we use window_handle to draw WebView on Windows, this will crash by:
This error caused by when used WebView2.
thread 'main' panicked at crates\gpui\src\app\async_context.rs:91:28:
already borrowed: BorrowMutError
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'main' panicked at library\core\src\panicking.rs:221:5:
panic in a function that cannot unwind
Try this https://github.com/tauri-apps/wry/pull/1383 on Windows can replay the crash.
In fact, we had done a similar fix around August last year, but we used the unsafe method to avoid crashes in that version, we felt that it was not a good change, so we do not make PR.
Today @sunli829 thought about it again and changed the method. Now using try_borrow_mut is similar to the previous borrow_mut.
https://github.com/zed-industries/zed/blob/691de6b4b36d3ada91a1e238904b065eec454188/crates/gpui/src/app.rs#L70-L78
I have tested to start Zed by those changes, it is looks no problem.
We require contributors to sign our Contributor License Agreement, and we don't have @sunli829 on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.
@cla-bot check
The cla-bot has been summoned, and re-checked this pull request!
Hello. I would be very excited to see the WebView support in Zed. Also because it would expose a Canvas, allowing the port of some mroe sophisticated VSCode extensions. Is the work still onging?
@JunkuiZhang @SomeoneToIgnore
This PR is important to provided us ability to use WebView or other depends raw_window_handle stuffs for works on Windows.
We are already useing this in our project and it works well in a long term.
Sorry, I have to push for this, because this PR is waited a bit long time.
Really sorry about this, I nearly forgot about this PR
While your PR mentions fixing a Windows issue, I tested it on macOS and it's failing there too with this error:
thread 'main' panicked at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/cocoa-0.26.0/src/appkit.rs:3441:9:
null pointer dereference occurred
Looks like this bug might need some more investigation.
How to do you start up? I have checked it again by start Zed in development cargo run, There is no error.
Please submit the full of error stack.
I’m talking about the example you provided and attempted to fix https://github.com/tauri-apps/wry/pull/1383
Simply run that example and it reliably crashes on macOS
I'll check it again tomorrow morning.
But we've been using this change to include WebView in our apps for a long time, and macOS is correct. It may be some other issue, such as what updates wry has recently made.
How to reproduce the crash
- Clone the repo: https://github.com/JunkuiZhang/wry
-
git switch test -
cargo run --example gpui -- --nocapture
Full backtrace
Finished `dev` profile [unoptimized + debuginfo] target(s) in 11.90s
Running `target/debug/examples/gpui --nocapture`
1
2
thread 'main' panicked at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/cocoa-0.26.0/src/appkit.rs:3441:9:
null pointer dereference occurred
stack backtrace:
0: rust_begin_unwind
at /rustc/05f9846f893b09a1be1fc8560e33fc3c815cfecb/library/std/src/panicking.rs:695:5
1: core::panicking::panic_nounwind_fmt::runtime
at /rustc/05f9846f893b09a1be1fc8560e33fc3c815cfecb/library/core/src/panicking.rs:117:22
2: core::panicking::panic_nounwind_fmt
at /rustc/05f9846f893b09a1be1fc8560e33fc3c815cfecb/library/core/src/intrinsics/mod.rs:3886:9
3: core::panicking::panic_null_pointer_dereference
at /rustc/05f9846f893b09a1be1fc8560e33fc3c815cfecb/library/core/src/panicking.rs:304:5
4: <*mut objc::runtime::Object as cocoa::appkit::NSScreen>::backingScaleFactor
at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/cocoa-0.26.0/src/appkit.rs:3441:9
5: gpui::platform::mac::window::get_scale_factor
at /Users/zhangjunkui/projects/zed/crates/gpui/src/platform/mac/window.rs:1199:9
6: gpui::platform::mac::window::MacWindowState::scale_factor
at /Users/zhangjunkui/projects/zed/crates/gpui/src/platform/mac/window.rs:482:9
7: gpui::platform::mac::window::set_frame_size
at /Users/zhangjunkui/projects/zed/crates/gpui/src/platform/mac/window.rs:1702:24
8: <unknown>
9: <unknown>
10: <(A,) as objc::message::MessageArguments>::invoke
at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/objc-0.2.7/src/message/mod.rs:128:17
11: objc::message::platform::send_unverified
at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/objc-0.2.7/src/message/apple/mod.rs:27:9
12: objc::message::send_message
at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/objc-0.2.7/src/message/mod.rs:178:5
13: <*mut objc::runtime::Object as cocoa::appkit::NSWindow>::setContentView_
at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/cocoa-0.26.0/src/appkit.rs:1445:9
14: gpui::platform::mac::window::MacWindow::open
at /Users/zhangjunkui/projects/zed/crates/gpui/src/platform/mac/window.rs:689:13
15: <gpui::platform::mac::platform::MacPlatform as gpui::platform::Platform>::open_window
at /Users/zhangjunkui/projects/zed/crates/gpui/src/platform/mac/platform.rs:583:21
16: gpui::window::Window::new
at /Users/zhangjunkui/projects/zed/crates/gpui/src/window.rs:724:35
17: gpui::app::App::open_window::{{closure}}
at /Users/zhangjunkui/projects/zed/crates/gpui/src/app.rs:608:19
18: gpui::app::App::update
at /Users/zhangjunkui/projects/zed/crates/gpui/src/app.rs:426:22
19: gpui::app::App::open_window
at /Users/zhangjunkui/projects/zed/crates/gpui/src/app.rs:605:9
20: gpui::main::{{closure}}
at ./examples/gpui.rs:226:18
21: gpui::app::Application::run::{{closure}}
at /Users/zhangjunkui/projects/zed/crates/gpui/src/app.rs:166:13
22: core::ops::function::FnOnce::call_once{{vtable.shim}}
at /Users/zhangjunkui/.rustup/toolchains/stable-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
23: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
at /Users/zhangjunkui/.rustup/toolchains/stable-x86_64-apple-darwin/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1976:9
24: gpui::platform::mac::platform::did_finish_launching
at /Users/zhangjunkui/projects/zed/crates/gpui/src/platform/mac/platform.rs:1334:13
25: <unknown>
26: <unknown>
27: <unknown>
28: <unknown>
29: <unknown>
30: <unknown>
31: <unknown>
32: <unknown>
33: <unknown>
34: <unknown>
35: <unknown>
36: <unknown>
37: <unknown>
38: <unknown>
39: <unknown>
40: <unknown>
41: <unknown>
42: <unknown>
43: <() as objc::message::MessageArguments>::invoke
at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/objc-0.2.7/src/message/mod.rs:128:17
44: objc::message::platform::send_unverified
at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/objc-0.2.7/src/message/apple/mod.rs:27:9
45: objc::message::send_message
at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/objc-0.2.7/src/message/mod.rs:178:5
46: <*mut objc::runtime::Object as cocoa::appkit::NSApplication>::run
at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/cocoa-0.26.0/src/appkit.rs:628:9
47: <gpui::platform::mac::platform::MacPlatform as gpui::platform::Platform>::run
at /Users/zhangjunkui/projects/zed/crates/gpui/src/platform/mac/platform.rs:454:13
48: gpui::app::Application::run
at /Users/zhangjunkui/projects/zed/crates/gpui/src/app.rs:164:9
49: gpui::main
at ./examples/gpui.rs:217:3
50: core::ops::function::FnOnce::call_once
at /Users/zhangjunkui/.rustup/toolchains/stable-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
thread caused non-unwinding panic. aborting.
[1] 95817 abort cargo run --example gpui -- --nocapture Finished `dev` profile [unoptimized + debuginfo] target(s) in 11.90s
Running `target/debug/examples/gpui --nocapture`
1
2
thread 'main' panicked at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/cocoa-0.26.0/src/appkit.rs:3441:9:
null pointer dereference occurred
stack backtrace:
0: rust_begin_unwind
at /rustc/05f9846f893b09a1be1fc8560e33fc3c815cfecb/library/std/src/panicking.rs:695:5
1: core::panicking::panic_nounwind_fmt::runtime
at /rustc/05f9846f893b09a1be1fc8560e33fc3c815cfecb/library/core/src/panicking.rs:117:22
2: core::panicking::panic_nounwind_fmt
at /rustc/05f9846f893b09a1be1fc8560e33fc3c815cfecb/library/core/src/intrinsics/mod.rs:3886:9
3: core::panicking::panic_null_pointer_dereference
at /rustc/05f9846f893b09a1be1fc8560e33fc3c815cfecb/library/core/src/panicking.rs:304:5
4: <*mut objc::runtime::Object as cocoa::appkit::NSScreen>::backingScaleFactor
at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/cocoa-0.26.0/src/appkit.rs:3441:9
5: gpui::platform::mac::window::get_scale_factor
at /Users/zhangjunkui/projects/zed/crates/gpui/src/platform/mac/window.rs:1199:9
6: gpui::platform::mac::window::MacWindowState::scale_factor
at /Users/zhangjunkui/projects/zed/crates/gpui/src/platform/mac/window.rs:482:9
7: gpui::platform::mac::window::set_frame_size
at /Users/zhangjunkui/projects/zed/crates/gpui/src/platform/mac/window.rs:1702:24
8: <unknown>
9: <unknown>
10: <(A,) as objc::message::MessageArguments>::invoke
at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/objc-0.2.7/src/message/mod.rs:128:17
11: objc::message::platform::send_unverified
at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/objc-0.2.7/src/message/apple/mod.rs:27:9
12: objc::message::send_message
at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/objc-0.2.7/src/message/mod.rs:178:5
13: <*mut objc::runtime::Object as cocoa::appkit::NSWindow>::setContentView_
at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/cocoa-0.26.0/src/appkit.rs:1445:9
14: gpui::platform::mac::window::MacWindow::open
at /Users/zhangjunkui/projects/zed/crates/gpui/src/platform/mac/window.rs:689:13
15: <gpui::platform::mac::platform::MacPlatform as gpui::platform::Platform>::open_window
at /Users/zhangjunkui/projects/zed/crates/gpui/src/platform/mac/platform.rs:583:21
16: gpui::window::Window::new
at /Users/zhangjunkui/projects/zed/crates/gpui/src/window.rs:724:35
17: gpui::app::App::open_window::{{closure}}
at /Users/zhangjunkui/projects/zed/crates/gpui/src/app.rs:608:19
18: gpui::app::App::update
at /Users/zhangjunkui/projects/zed/crates/gpui/src/app.rs:426:22
19: gpui::app::App::open_window
at /Users/zhangjunkui/projects/zed/crates/gpui/src/app.rs:605:9
20: gpui::main::{{closure}}
at ./examples/gpui.rs:226:18
21: gpui::app::Application::run::{{closure}}
at /Users/zhangjunkui/projects/zed/crates/gpui/src/app.rs:166:13
22: core::ops::function::FnOnce::call_once{{vtable.shim}}
at /Users/zhangjunkui/.rustup/toolchains/stable-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
23: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
at /Users/zhangjunkui/.rustup/toolchains/stable-x86_64-apple-darwin/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1976:9
24: gpui::platform::mac::platform::did_finish_launching
at /Users/zhangjunkui/projects/zed/crates/gpui/src/platform/mac/platform.rs:1334:13
25: <unknown>
26: <unknown>
27: <unknown>
28: <unknown>
29: <unknown>
30: <unknown>
31: <unknown>
32: <unknown>
33: <unknown>
34: <unknown>
35: <unknown>
36: <unknown>
37: <unknown>
38: <unknown>
39: <unknown>
40: <unknown>
41: <unknown>
42: <unknown>
43: <() as objc::message::MessageArguments>::invoke
at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/objc-0.2.7/src/message/mod.rs:128:17
44: objc::message::platform::send_unverified
at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/objc-0.2.7/src/message/apple/mod.rs:27:9
45: objc::message::send_message
at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/objc-0.2.7/src/message/mod.rs:178:5
46: <*mut objc::runtime::Object as cocoa::appkit::NSApplication>::run
at /Users/zhangjunkui/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/cocoa-0.26.0/src/appkit.rs:628:9
47: <gpui::platform::mac::platform::MacPlatform as gpui::platform::Platform>::run
at /Users/zhangjunkui/projects/zed/crates/gpui/src/platform/mac/platform.rs:454:13
48: gpui::app::Application::run
at /Users/zhangjunkui/projects/zed/crates/gpui/src/app.rs:164:9
49: gpui::main
at ./examples/gpui.rs:217:3
50: core::ops::function::FnOnce::call_once
at /Users/zhangjunkui/.rustup/toolchains/stable-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
thread caused non-unwinding panic. aborting.
[1] 95817 abort cargo run --example gpui -- --nocapture
BTW, I suspect this bug might be related to recent rust updates, it looks very similar to this one. While adding null checks would be a quick fix, what's puzzling to me is why we're hitting a null pointer during creation in the first place.
Previous null pointer bugs we fixed were all environment-specific (like minimized windows), but getting one during initialization feels abnormal. There might be a deeper issue here that needs investigation beyond just the null check.
That should be caused by it, we used GPUI version that not include #28599, I remember that I had update in last week.
https://github.com/huacnlee/zed/pull/16#event-17104923619
I have rebased main branch latest code.
It is strange, I'm running fine.
~~I have added webview example as a commit into this PR. When later we have confirmed, we can reset to avoid merge webview example commit.~~
Today, I hit this crash as the comment, After I add this code,
index 26a62aeadf..c49219bff1 100644
--- a/crates/gpui/src/platform/mac/window.rs
+++ b/crates/gpui/src/platform/mac/window.rs
@@ -844,6 +844,9 @@ impl PlatformWindow for MacWindow {
fn display(&self) -> Option<Rc<dyn PlatformDisplay>> {
unsafe {
let screen = self.0.lock().native_window.screen();
+ if screen.is_null() {
+ return None;
+ }
let device_description: id = msg_send![screen, deviceDescription];
let screen_number: id = NSDictionary::valueForKey_(
device_description,
@@ -1193,6 +1196,9 @@ impl rwh::HasDisplayHandle for MacWindow {
fn get_scale_factor(native_window: id) -> f32 {
let factor = unsafe {
let screen: id = msg_send![native_window, screen];
+ if screen.is_null() {
+ return 1.0;
+ }
NSScreen::backingScaleFactor(screen) as f32
};
I thought I fixed this problem, But the interesting things, is that, when I reset this modify code, rebuild to get the crash report, everything work well. I waste one more hour.
So I think this crash report might not block this PR.
I don't known if I should send a PR for this code.
I think, we can merge this PR now, @CharlesChen0823 have fixed crash issue, it is not relative on this PR changes. @SomeoneToIgnore @JunkuiZhang
Like I mentioned in my earlier comment, I don't think just checking for a null pointer is a solid fix. The bigger issue here is that, for some reason, other parts of the code are running while the window is still being created (on macOS and Windows). Since I don’t have the bandwidth to fully debug this right now, I’ve decided to go ahead and merge this PR as-is.
Thanks BTW