Fix panic when destroying window

[xiaopengli89]

• Tested on all platforms changed
• Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users

Dropping window may cause panic.

thread 'main' panicked at 'view to have a window', src/platform_impl/macos/view.rs:880:32

[madsmtm]

Huh, I would have thought this to be impossible, since view methods should not be called if the view is no longer attached to a window?

Are you encountering this in multi-threaded code? If so, I suspect that may be the real culprit instead?

[xiaopengli89]

Here is a minimum reproducible example:

use winit::{
    event::{Event, WindowEvent},
    event_loop::EventLoop,
    window::WindowBuilder,
};

fn main() {
    let event_loop = EventLoop::new();

    let mut window = WindowBuilder::new()
        .with_title("A fantastic window!")
        .with_inner_size(winit::dpi::LogicalSize::new(128.0, 128.0))
        .build(&event_loop)
        .unwrap();

    event_loop.run(move |event, target, control_flow| {
        control_flow.set_wait();

        match event {
            Event::WindowEvent {
                event: WindowEvent::CloseRequested,
                window_id: _,
            } => control_flow.set_exit(),
            Event::MainEventsCleared => {
                window.request_redraw();

                window = WindowBuilder::new()
                    .with_title("A fantastic window!")
                    .with_inner_size(winit::dpi::LogicalSize::new(128.0, 128.0))
                    .build(target)
                    .unwrap();
            }
            _ => (),
        }
    });
}

[kchibisov]

This will need a rebase.

ping @madsmtm for re-review

[xiaopengli89]

Done.

[madsmtm]

Hmm, I cannot reproduce from your example? Is it still an issue for you?

Maybe you can add #[track_caller] to window and window_id, or post the full stack-trace, so that we can see which call is the real culprit?

[xiaopengli89]

Hmm, I cannot reproduce from your example? Is it still an issue for you?

Maybe you can add #[track_caller] to window and window_id, or post the full stack-trace, so that we can see which call is the real culprit?

[05:25 pm] winit on  master [!]
❯ let-env RUST_BACKTRACE = 1

[05:25 pm] winit on  master [!]
❯ cargo r --example window
    Finished dev [unoptimized + debuginfo] target(s) in 0.07s
     Running `target/debug/examples/window`
thread 'main' panicked at 'view to have a window', src/platform_impl/macos/view.rs:811:32
stack backtrace:
   0: rust_begin_unwind
             at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/std/src/panicking.rs:593:5
   1: core::panicking::panic_fmt
             at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/core/src/panicking.rs:67:14
   2: core::panicking::panic_display
             at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/core/src/panicking.rs:150:5
   3: core::panicking::panic_str
             at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/core/src/panicking.rs:134:5
   4: core::option::expect_failed
             at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/core/src/option.rs:1932:5
   5: core::option::Option<T>::expect
             at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/core/src/option.rs:898:21
   6: winit::platform_impl::platform::view::WinitView::window
             at ./src/platform_impl/macos/view.rs:811:9
   7: winit::platform_impl::platform::view::WinitView::window_id
             at ./src/platform_impl/macos/view.rs:815:18
   8: winit::platform_impl::platform::view::WinitView::draw_rect
             at ./src/platform_impl/macos/view.rs:246:37
   9: <unknown>
  10: <unknown>
  11: <unknown>
  12: <unknown>
  13: <unknown>
  14: <unknown>
  15: <unknown>
  16: <unknown>
  17: <unknown>
  18: <unknown>
  19: <unknown>
  20: <unknown>
  21: <unknown>
  22: <unknown>
  23: <unknown>
  24: <unknown>
  25: <unknown>
  26: <unknown>
  27: <unknown>
  28: <unknown>
  29: <unknown>
  30: <unknown>
  31: <unknown>
  32: <() as objc2::message::MessageArguments>::__invoke
             at /Users/admin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/objc2-0.3.0-beta.3/src/message/mod.rs:390:26
  33: objc2::message::platform::send_unverified::{{closure}}
             at /Users/admin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/objc2-0.3.0-beta.3/src/message/apple/mod.rs:36:33
  34: objc2::message::conditional_try
             at /Users/admin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/objc2-0.3.0-beta.3/src/message/mod.rs:30:5
  35: objc2::message::platform::send_unverified
             at /Users/admin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/objc2-0.3.0-beta.3/src/message/apple/mod.rs:36:14
  36: objc2::message::MessageReceiver::send_message
             at /Users/admin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/objc2-0.3.0-beta.3/src/message/mod.rs:195:46
  37: winit::platform_impl::platform::appkit::application::NSApplication::run
             at /Users/admin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/objc2-0.3.0-beta.3/src/macros/__rewrite_self_arg.rs:42:21
  38: winit::platform_impl::platform::event_loop::EventLoop<T>::run_return::{{closure}}
             at ./src/platform_impl/macos/event_loop.rs:220:22
  39: objc2::rc::autorelease::autoreleasepool
             at /Users/admin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/objc2-0.3.0-beta.3/src/rc/autorelease.rs:313:5
  40: winit::platform_impl::platform::event_loop::EventLoop<T>::run_return
             at ./src/platform_impl/macos/event_loop.rs:211:25
  41: winit::platform_impl::platform::event_loop::EventLoop<T>::run
             at ./src/platform_impl/macos/event_loop.rs:190:25
  42: winit::event_loop::EventLoop<T>::run
             at ./src/event_loop.rs:308:9
  43: window::main
             at ./examples/window.rs:16:5
  44: core::ops::function::FnOnce::call_once
             at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

[madsmtm]

Interesting! Now I'm wondering why drawRect: is being called!

May be part of #2640, since we're handling redraws kinda badly right now?
If you only change drawRect: to be something like the following, does the issue appear elsewhere?

trace_scope!("drawRect:");
let Some(window) = self._ns_window.load() {
    AppState::handle_redraw(window.id());
}
let _: () = unsafe { msg_send![super(self), drawRect: rect] };

And maybe you could enable tracing as well, would be nice for me to see the events you're hitting before the crash.

[xiaopengli89]

If you only change drawRect: to be something like the following, does the issue appear elsewhere?

The issue resolved, so is it a better solution?

[madsmtm]

I think I would prefer that, since I suspect it is something that ties in with the fix for #2640.

[xiaopengli89]

Done.

[madsmtm]

Thanks for the contribution!