Race condition: run_forever can dispatch events after the window is destroyed

[SimonSapin]

Background

While working on rust-windowing/glutin#998, my test program was crashing every time I closed the window:

Assertion failed: false, file gfx/angle/checkout/src/libANGLE/renderer/d3d/SurfaceD3D.cpp, line 307
error: process didn't exit successfully: `target\debug\mozangle-with-glutin-example.exe` (exit code: 3221226505)

This is ANGLE somewhere inside eglSwapBuffers, calling Windows’s GetClientRect with the pointer returned by glutin::Window::platform_window that Glutin had previously passed to eglCreateWindowSurface, and asserting that this call succeeds. Here it was failing, with a debugger showing that GetLastError() returned 1400 which is ERROR_INVALID_WINDOW_HANDLE and showing indeed the that pointer as "unreadable memory".

Conditions

Trying to reproduce this without ANGLE, I found out after a while that this problem only happens when:

1. The closure passed to events_loop.run_forever(…) does some non-trivial work for each event
2. The cursor is moved over the window, such that a bunch of events are queued
3. The "close window" button is clicked after that, without waiting.

As a result the closure keeps being called to handle events, even though the window is already gone and the HWND pointer is invalid.

I don’t know if this problem is Windows-specific or if a similar race condition happens on other platforms.

Test case

• Steps: run the program below on Windows, mouse over the window for a bit, then close it within a couple seconds.

• Actual result: GetClientRect failed. GetLastError: 1400 is printed dozens of times.

• Expected result: no output, GetClientRect always succeeds. Either the OS window is kept alive until all events are dispatched (or maybe as long as the winit::Window exists), or queued events are dropped when the window is destroyed.

  Alternatively, winit::Window gains a new method that tests whether the OS window still exists, and docs and examples point out that is should be used before doing anything with the window. (Maybe glutin::GlWindow::swap_buffers also uses it to become a no-op on a closed window.)

extern crate winapi;
extern crate winit;

use winit::os::windows::WindowExt;

fn main() {
    let mut events_loop = winit::EventsLoop::new();

    let window = winit::WindowBuilder::new()
        .build(&events_loop)
        .unwrap();

    events_loop.run_forever(|event| {
//        println!("{:?}", event);

        match event {
            winit::Event::WindowEvent { event: winit::WindowEvent::Closed, .. } => {
                return winit::ControlFlow::Break
            },
            _ => {}
        }
        unsafe {
            let hwnd = window.get_hwnd() as _;
            let mut rect = winapi::shared::windef::RECT { left: 0, top: 0, right: 0, bottom: 0 };
            if winapi::um::winuser::GetClientRect(hwnd, &mut rect) == 0 {
                println!("GetClientRect failed. GetLastError: {:?}",
                         winapi::um::errhandlingapi::GetLastError());
            }
        }
        std::thread::sleep(std::time::Duration::from_millis(10)); // Pretend to do some work
        winit::ControlFlow::Continue
    });
}

[tomaka]

This is most likely a win32-only issue.
If the same thing happens on another platform, it should be a different issue.

[Osspial]

Closing since, while the fix isn't quite on master, #638 addresses this.