On Windows, fix bug where we'd try to emit MainEventsCleared events during nested win32 event loops (#1615)

CHANGELOG.md

@@ -1,4 +1,5 @@
 # Unreleased
+
 - On Unix, X11 and Wayland are now optional features (enabled by default)
 - On X11, fix deadlock when calling `set_fullscreen_inner`.
 - On Web, prevent the webpage from scrolling when the user is focused on a winit canvas
@@ -7,6 +8,7 @@
 - On macOS, add `hide__other_applications` to `EventLoopWindowTarget` via existing `EventLoopWindowTargetExtMacOS` trait. `hide_other_applications` will hide other applications by calling `-[NSApplication hideOtherApplications: nil]`.
 - On android added support for `run_return`.
 - On MacOS, Fixed fullscreen and dialog support for `run_return`.
+- On Windows, fix bug where we'd try to emit `MainEventsCleared` events during nested win32 event loops.
 
 # 0.22.2 (2020-05-16)
 

src/platform_impl/windows/event_loop.rs

@@ -226,7 +226,7 @@ impl<T: 'static> EventLoop<T> {
         }
 
         unsafe {
-            runner.call_event_handler(Event::LoopDestroyed);
+            runner.loop_destroyed();
         }
         runner.reset_runner();
     }
@@ -1933,14 +1933,25 @@ unsafe extern "system" fn thread_event_target_callback<T: 'static>(
             // events, `handling_events` will return false and we won't emit a second
             // `RedrawEventsCleared` event.
             if subclass_input.event_loop_runner.handling_events() {
-                // This WM_PAINT handler will never be re-entrant because `flush_paint_messages`
-                // doesn't call WM_PAINT for the thread event target (i.e. this window).
-                assert!(flush_paint_messages(
-                    None,
-                    &subclass_input.event_loop_runner
-                ));
-                subclass_input.event_loop_runner.redraw_events_cleared();
-                process_control_flow(&subclass_input.event_loop_runner);
+                if subclass_input.event_loop_runner.should_buffer() {
+                    // This branch can be triggered when a nested win32 event loop is triggered
+                    // inside of the `event_handler` callback.
+                    winuser::RedrawWindow(
+                        window,
+                        ptr::null(),
+                        ptr::null_mut(),
+                        winuser::RDW_INTERNALPAINT,
+                    );
+                } else {
+                    // This WM_PAINT handler will never be re-entrant because `flush_paint_messages`
+                    // doesn't call WM_PAINT for the thread event target (i.e. this window).
+                    assert!(flush_paint_messages(
+                        None,
+                        &subclass_input.event_loop_runner
+                    ));
+                    subclass_input.event_loop_runner.redraw_events_cleared();
+                    process_control_flow(&subclass_input.event_loop_runner);
+                }
             }
 
             0

src/platform_impl/windows/event_loop/runner.rs

@@ -53,6 +53,8 @@ enum RunnerState {
     /// The event loop is handling the redraw events and sending them to the user's callback.
     /// `MainEventsCleared` has been sent, and `RedrawEventsCleared` hasn't.
     HandlingRedrawEvents,
+    /// The event loop has been destroyed. No other events will be emitted.
+    Destroyed,
 }
 
 enum BufferedEvent<T: 'static> {
@@ -229,7 +231,11 @@ impl<T> EventLoopRunner<T> {
         self.move_state_to(RunnerState::Idle);
     }
 
-    pub(crate) unsafe fn call_event_handler(&self, event: Event<'_, T>) {
+    pub(crate) unsafe fn loop_destroyed(&self) {
+        self.move_state_to(RunnerState::Destroyed);
+    }
+
+    unsafe fn call_event_handler(&self, event: Event<'_, T>) {
         self.catch_unwind(|| {
             let mut control_flow = self.control_flow.take();
             let mut event_handler = self.event_handler.take()
@@ -260,8 +266,8 @@ impl<T> EventLoopRunner<T> {
         }
     }
 
-    /// Dispatch control flow events (`NewEvents`, `MainEventsCleared`, and `RedrawEventsCleared`) as
-    /// necessary to bring the internal `RunnerState` to the new runner state.
+    /// Dispatch control flow events (`NewEvents`, `MainEventsCleared`, `RedrawEventsCleared`, and
+    /// `LoopDestroyed`) as necessary to bring the internal `RunnerState` to the new runner state.
     ///
     /// The state transitions are defined as follows:
     ///
@@ -273,14 +279,20 @@ impl<T> EventLoopRunner<T> {
     ///   ^            |
     ///   |            V
     /// Idle <--- HandlingRedrawEvents
+    ///   |
+    ///   V
+    /// Destroyed
     /// ```
     ///
-    /// Attempting to transition back to `Uninitialized` will result in a panic. Transitioning to
-    /// the current state is a no-op. Even if the `new_runner_state` isn't the immediate next state
-    /// in the runner state machine (e.g. `self.runner_state == HandlingMainEvents` and
+    /// Attempting to transition back to `Uninitialized` will result in a panic. Attempting to
+    /// transition *from* `Destroyed` will also reuslt in a panic. Transitioning to the current
+    /// state is a no-op. Even if the `new_runner_state` isn't the immediate next state in the
+    /// runner state machine (e.g. `self.runner_state == HandlingMainEvents` and
     /// `new_runner_state == Idle`), the intermediate state transitions will still be executed.
     unsafe fn move_state_to(&self, new_runner_state: RunnerState) {
-        use RunnerState::{HandlingMainEvents, HandlingRedrawEvents, Idle, Uninitialized};
+        use RunnerState::{
+            Destroyed, HandlingMainEvents, HandlingRedrawEvents, Idle, Uninitialized,
+        };
 
         match (
             self.runner_state.replace(new_runner_state),
@@ -289,7 +301,8 @@ impl<T> EventLoopRunner<T> {
             (Uninitialized, Uninitialized)
             | (Idle, Idle)
             | (HandlingMainEvents, HandlingMainEvents)
-            | (HandlingRedrawEvents, HandlingRedrawEvents) => (),
+            | (HandlingRedrawEvents, HandlingRedrawEvents)
+            | (Destroyed, Destroyed) => (),
 
             // State transitions that initialize the event loop.
             (Uninitialized, HandlingMainEvents) => {
@@ -304,6 +317,12 @@ impl<T> EventLoopRunner<T> {
                 self.call_event_handler(Event::MainEventsCleared);
                 self.call_redraw_events_cleared();
             }
+            (Uninitialized, Destroyed) => {
+                self.call_new_events(true);
+                self.call_event_handler(Event::MainEventsCleared);
+                self.call_redraw_events_cleared();
+                self.call_event_handler(Event::LoopDestroyed);
+            }
             (_, Uninitialized) => panic!("cannot move state to Uninitialized"),
 
             // State transitions that start the event handling process.
@@ -314,6 +333,9 @@ impl<T> EventLoopRunner<T> {
                 self.call_new_events(false);
                 self.call_event_handler(Event::MainEventsCleared);
             }
+            (Idle, Destroyed) => {
+                self.call_event_handler(Event::LoopDestroyed);
+            }
 
             (HandlingMainEvents, HandlingRedrawEvents) => {
                 self.call_event_handler(Event::MainEventsCleared);
@@ -323,6 +345,11 @@ impl<T> EventLoopRunner<T> {
                 self.call_event_handler(Event::MainEventsCleared);
                 self.call_redraw_events_cleared();
             }
+            (HandlingMainEvents, Destroyed) => {
+                self.call_event_handler(Event::MainEventsCleared);
+                self.call_redraw_events_cleared();
+                self.call_event_handler(Event::LoopDestroyed);
+            }
 
             (HandlingRedrawEvents, Idle) => {
                 self.call_redraw_events_cleared();
@@ -332,6 +359,12 @@ impl<T> EventLoopRunner<T> {
                 self.call_redraw_events_cleared();
                 self.call_new_events(false);
             }
+            (HandlingRedrawEvents, Destroyed) => {
+                self.call_redraw_events_cleared();
+                self.call_event_handler(Event::LoopDestroyed);
+            }
+
+            (Destroyed, _) => panic!("cannot move state from Destroyed"),
         }
     }