Window not destroyed after run_and_return

[flukejones]

Describe the bug
Somewhere between version 0.20.1 and version 0.21.0 the ability to re-run an app with run_and_return was broken such that the window is no-longer destroyed (using eframe).

This can be reproduced using the serial windows example. In version 0.20.1 this example can close and destroy the window when the window bar X is clicked, but not when the "close" button is clicked. In version 0.21.x clicking the window X results in the same behaviour as the "close" button.

I initially thought it might be a winit issue but looking at the related issues there shows that egui is already using the found "workarounds".

A git checkout 0.20.1 -- ./crates/eframe shows that the issue lies within eframe

[flukejones]

Narrowed it down to commit cb77458 with git bisect.

cb77458f70c900c5e6c941544ae114281065b2f8 is the first bad commit
commit cb77458f70c900c5e6c941544ae114281065b2f8
Author: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
Date:   Mon Dec 12 15:16:32 2022 +0100

    eframe error handling (#2433)
    
    * eframe::run_native: return errors instead of crashing
    
    * Detect and handle glutin errors
    
    * egui_demo_app: silence wgpu log spam
    
    * Add trace logs for why eframe is shutting down
    
    * Fix: only save App state once on Mac
    
    * Handle Winit failure
    
    * Log where we load app state from
    
    * Don't panic on zero-sized window
    
    * Clamp loaded window size to not be too tiny to see
    
    * Simplify code: more shared code in window_builder
    
    * Improve code readability
    
    * Fix wasm32 build
    
    * fix android
    
    * Update changelog

 Cargo.lock                                  |   1 +
 crates/eframe/CHANGELOG.md                  |   3 +
 crates/eframe/Cargo.toml                    |   1 +
 crates/eframe/src/epi.rs                    |   1 +
 crates/eframe/src/lib.rs                    |  39 ++++-
 crates/eframe/src/native/epi_integration.rs |  23 ++-
 crates/eframe/src/native/file_storage.rs    |   1 +
 crates/eframe/src/native/run.rs             | 238 +++++++++++++++-------------
 crates/egui-wgpu/CHANGELOG.md               |   2 +-
 crates/egui-winit/src/window_settings.rs    |  21 ++-
 crates/egui_demo_app/src/main.rs            |  15 +-
 crates/egui_glow/examples/pure_glow.rs      |   4 +-
 examples/confirm_exit/src/main.rs           |   4 +-
 examples/custom_3d_glow/src/main.rs         |   4 +-
 examples/custom_3d_three-d/src/main.rs      |   4 +-
 examples/custom_font/src/main.rs            |   4 +-
 examples/custom_font_style/src/main.rs      |   4 +-
 examples/custom_window_frame/src/main.rs    |   4 +-
 examples/download_image/src/main.rs         |   4 +-
 examples/file_dialog/src/main.rs            |   4 +-
 examples/hello_world/src/main.rs            |   4 +-
 examples/keyboard_events/src/main.rs        |   4 +-
 examples/puffin_profiler/src/main.rs        |   4 +-
 examples/retained_image/src/main.rs         |   4 +-
 examples/screenshot/src/main.rs             |   4 +-
 examples/serial_windows/src/main.rs         |   8 +-
 examples/svg/src/main.rs                    |   4 +-
 27 files changed, 250 insertions(+), 163 deletions(-)

[flukejones]

Got it:

diff --git a/crates/eframe/src/native/run.rs b/crates/eframe/src/native/run.rs
index a25ed4edd3b6..cfa047bbd812 100644
--- a/crates/eframe/src/native/run.rs
+++ b/crates/eframe/src/native/run.rs
@@ -175,6 +175,7 @@ fn run_and_return(
             }
             EventResult::Exit => {
                 tracing::debug!("Asking to exit event loop…");
+                winit_app.save_and_destroy();
                 *control_flow = ControlFlow::Exit;
                 return;
             }

Without this call the window refuses to destroy itself.

From what I can see this call is hit before the one above it in:

            winit::event::Event::LoopDestroyed => {
                // On Mac, Cmd-Q we get here and then `run_return` doesn't return (despite its name),
                // so we need to save state now:
                tracing::debug!("Received Event::LoopDestroyed - saving app state…");
                winit_app.save_and_destroy();
                *control_flow = ControlFlow::Exit;
                return;
            }

and in fact reverting the above block to:

            winit::event::Event::LoopDestroyed => {
                tracing::debug!("winit::event::Event::LoopDestroyed");
                EventResult::Exit
            }

has no effect on the issue, or app behaviour for me.

I'm taking a flying guess here and suspecting that the root cause of my issue is in the order of operations perhaps - where control_flow is being set, and the event loop in winit is doing stuff before we tick back over to the start and reach winit::event::Event::LoopDestroyed.

I'll do a PR.

[wlwatkins]

Hi, I still get the issue with egui 0.27. When wlosing the main window, the logs do show that the main viewport should be destroyed, but it isn't. (WIndows 11. Rust 1.79)

2024-04-08T14:53:45.875872Z DEBUG eframe::native::run: 176: Asking to exit event loop…
2024-04-08T14:53:45.891124Z DEBUG eframe::native::run: 89: Received Event::LoopExiting - saving app state…    
2024-04-08T14:53:45.891610Z DEBUG eframe::native::run: 213: eframe window closed

would they be a manuel aproach? maybe using on_exit