flutter icon indicating copy to clipboard operation
flutter copied to clipboard

Windows, multi-window, crash on creating and closing window

Open fufesou opened this issue 1 year ago • 18 comments

Steps to reproduce

Related issue

https://github.com/MixinNetwork/flutter-plugins/issues/352

Run the demo https://github.com/lukasz-lukasz-lukasz/flutter-plugins/commit/4ae8e28318187ee8936ffcccdd37c3d433e53f15

Expected results

No crash

Actual results

Crashed

750fe5ec35cac51eeeb50741db6a696

image

Code sample

Code sample

https://github.com/lukasz-lukasz-lukasz/flutter-plugins/commit/4ae8e28318187ee8936ffcccdd37c3d433e53f15

Screenshots or Video

Screenshots / Video demonstration

[Upload media here]

Logs

Maybe the same cause https://github.com/flutter/flutter/issues/137973#issuecomment-1799326980

I suspect the proper fix is to make the FlutterViewController post a task to the raster thread to set the swap interval. This would ensure that the platform thread never makes the EGL context current during a rendering operation. I'll take a look at this.

@loic-sharma Could you please confirm?

Flutter Doctor output

Doctor output
[√] Flutter (Channel stable, 3.24.3, on Microsoft Windows [Version 10.0.20348.2655], locale en-US)
    • Flutter version 3.24.3 on channel stable at C:\workspace\flutter\flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 2663184aa7 (2 weeks ago), 2024-09-11 16:27:48 -0500
    • Engine revision 36335019a8
    • Dart version 3.5.3
    • DevTools version 2.37.3

[√] Windows Version (Installed version of Windows is version 10 or higher)

[X] Android toolchain - develop for Android devices
    X Unable to locate Android SDK.
      Install Android Studio from: https://developer.android.com/studio/index.html
      On first launch it will assist you in installing the Android SDK components.
      (or visit https://flutter.dev/to/windows-android-setup for detailed instructions).
      If the Android SDK has been installed to a custom location, please use
      `flutter config --android-sdk` to update to that location.


[√] Chrome - develop for the web
    • Chrome at C:\Program Files\Google\Chrome\Application\chrome.exe

[√] Visual Studio - develop Windows apps (Visual Studio Community 2022 17.11.4)
    • Visual Studio at C:\Program Files\Microsoft Visual Studio\2022\Community
    • Visual Studio Community 2022 version 17.11.35312.102
    • Windows 10 SDK version 10.0.22621.0

[!] Android Studio (not installed)
    • Android Studio not found; download from https://developer.android.com/studio/index.html
      (or visit https://flutter.dev/to/windows-android-setup for detailed instructions).

[√] VS Code (version 1.93.1)
    • VS Code at C:\Users\Administrator\AppData\Local\Programs\Microsoft VS Code
    • Flutter extension version 3.96.0

[√] Connected device (3 available)
    • Windows (desktop) • windows • windows-x64    • Microsoft Windows [Version 10.0.20348.2655]
    • Chrome (web)      • chrome  • web-javascript • Google Chrome 129.0.6668.71
    • Edge (web)        • edge    • web-javascript • Microsoft Edge 129.0.2792.52

[√] Network resources
    • All expected network resources are available.

! Doctor found issues in 2 categories.

fufesou avatar Sep 25 '24 12:09 fufesou

Hi @fufesou, can you test this on the master channel to see if you still experience this issue? I just tested on the master channel and I'm on window 155 (at the time of writing, still counting) and there hasn't been a crash yet.

danagbemava-nc avatar Sep 25 '24 13:09 danagbemava-nc

Sure, I'll test the master channel.

I'm on window 155 (at the time of writing, still counting) and there hasn't been a crash yet.

Yes, this is a random issue, you may still have to wait.

fufesou avatar Sep 25 '24 13:09 fufesou

Master channel also crashes, but I cannot see the call stack.

I'll build the engine(master) and test again.

fufesou avatar Sep 25 '24 15:09 fufesou

the master channel

Still crash by the same cause

fufesou avatar Sep 25 '24 23:09 fufesou

@fufesou Thanks for update, how long did it usually take for it crash for you on the master channel?

danagbemava-nc avatar Sep 26 '24 06:09 danagbemava-nc

Sometimes a dozen seconds, sometimes several minutes.

fufesou avatar Sep 26 '24 07:09 fufesou

Maybe the same cause #137973 (comment)

I suspect the proper fix is to make the FlutterViewController post a task to the raster thread to set the swap interval. This would ensure that the platform thread never makes the EGL context current during a rendering operation. I'll take a look at this.

@loic-sharma Could you please confirm?

This looks like a separate bug to me.

@fufesou Does this crash happen when you close the window? Perhaps there's a bug when the UI thread is closing a window and the raster thread is attempting to present to that same window. FYI, multi-window has been deprioritized so I likely won't have the bandwidth to investigate this myself. I'm happy to answer any questions and help you debug though! Sorry :(

loic-sharma avatar Sep 27 '24 00:09 loic-sharma

Thanks!

Does this crash happen when you close the window?

I'm not sure.

I'm currently working on something else.

@danagbemava-nc Please keep this issue open. I will continue working on it in a few days.

fufesou avatar Sep 27 '24 09:09 fufesou

Hi @fufesou, does it still reproduce easily for you on the latest master? I just tested master 3.27.0-1.0.pre.35 and it has been running for a while now. So far 1240 windows have been created/closed without any issue.

What are the specs of the device this reproduces easily for you on?

danagbemava-nc avatar Oct 14 '24 07:10 danagbemava-nc

Yes. I use the master branch to test.

fufesou avatar Oct 14 '24 11:10 fufesou

Hi @fufesou, please share the flutter doctor -v output of the master channel version you're using.

What are the specs of the device this reproduces easily for you on?

Please also share this information

danagbemava-nc avatar Oct 15 '24 05:10 danagbemava-nc

What are the specs of the device this reproduces easily for you on?

Sorry, I didn't see it.

This may be the most important difference.

I use the cloud server.

63a35e93a158 900e04e90b84

Others

Flutter doctor -v
[√] Flutter (Channel stable, 3.24.3, on Microsoft Windows [Version 10.0.20348.2655], locale en-US)
    • Flutter version 3.24.3 on channel stable at C:\workspace\flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 2663184aa7 (5 weeks ago), 2024-09-11 16:27:48 -0500
    • Engine revision 36335019a8
    • Dart version 3.5.3
    • DevTools version 2.37.3

[√] Windows Version (Installed version of Windows is version 10 or higher)

[X] Android toolchain - develop for Android devices
    X Unable to locate Android SDK.
      Install Android Studio from: https://developer.android.com/studio/index.html
      On first launch it will assist you in installing the Android SDK components.
      (or visit https://flutter.dev/to/windows-android-setup for detailed instructions).
      If the Android SDK has been installed to a custom location, please use
      `flutter config --android-sdk` to update to that location.


[√] Chrome - develop for the web
    • Chrome at C:\Program Files\Google\Chrome\Application\chrome.exe

[√] Visual Studio - develop Windows apps (Visual Studio Community 2022 17.11.5)
    • Visual Studio at C:\Program Files\Microsoft Visual Studio\2022\Community
    • Visual Studio Community 2022 version 17.11.35327.3
    • Windows 10 SDK version 10.0.22621.0

[!] Android Studio (not installed)
    • Android Studio not found; download from https://developer.android.com/studio/index.html
      (or visit https://flutter.dev/to/windows-android-setup for detailed instructions).

[√] VS Code (version 1.94.1)
    • VS Code at C:\Users\Administrator\AppData\Local\Programs\Microsoft VS Code
    • Flutter extension can be installed from:
       https://marketplace.visualstudio.com/items?itemName=Dart-Code.flutter

[√] Connected device (3 available)
    • Windows (desktop) • windows • windows-x64    • Microsoft Windows [Version 10.0.20348.2655]
    • Chrome (web)      • chrome  • web-javascript • Google Chrome 129.0.6668.101
    • Edge (web)        • edge    • web-javascript • Microsoft Edge 129.0.2792.79

[√] Network resources
    • All expected network resources are available.

! Doctor found issues in 2 categories.

But I use the master engine branch.

I build the engine myself and run flutter run -d windows --local-engine=C:\workspace\engine\src\out\host_debug --local-engine-host=host_debug

The commit hash is

commit 07d01ad1199522fa5889a10c1688c4e1812b6625 (HEAD, origin/main, origin/HEAD, main)
Author: skia-flutter-autoroll <[email protected]>
Date:   Thu Oct 10 00:50:32 2024 -0400

fufesou avatar Oct 15 '24 06:10 fufesou

Thanks.

Labeling for further investigation as I cannot reproduce this locally

danagbemava-nc avatar Oct 15 '24 13:10 danagbemava-nc

Thanks.

Labeling for further investigation as I cannot reproduce this locally

Hi @danagbemava-nc, I found that I could reproduce this problem stably by clicking the test button multiple times. After clicking it five times, the program would crash quickly.

Code sample

https://github.com/lukasz-lukasz-lukasz/flutter-plugins/commit/4ae8e28318187ee8936ffcccdd37c3d433e53f15

Flutter Doctor output

[flutter] flutter doctor -v Flutter assets will be downloaded from https://storage.flutter-io.cn. Make sure you trust this source! [√] Flutter (Channel stable, 3.24.3, on Microsoft Windows [版本 10.0.22631.4317], locale zh-CN) • Flutter version 3.24.3 on channel stable at C:\Users\vitoway\Downloads\flutter_windows_3.24.3-stable\flutter • Upstream repository https://github.com/flutter/flutter.git • Framework revision 2663184aa7 (5 weeks ago), 2024-09-11 16:27:48 -0500 • Engine revision 36335019a8 • Dart version 3.5.3 • DevTools version 2.37.3 • Pub download mirror https://pub.flutter-io.cn • Flutter download mirror https://storage.flutter-io.cn

[√] Windows Version (Installed version of Windows is version 10 or higher)

[X] Android toolchain - develop for Android devices X Unable to locate Android SDK. Install Android Studio from: https://developer.android.com/studio/index.html On first launch it will assist you in installing the Android SDK components. (or visit https://flutter.dev/to/windows-android-setup for detailed instructions). If the Android SDK has been installed to a custom location, please use flutter config --android-sdk to update to that location.

[√] Chrome - develop for the web • Chrome at C:\Program Files\Google\Chrome\Application\chrome.exe

[√] Visual Studio - develop Windows apps (Visual Studio Community 2022 17.11.5) • Visual Studio at C:\Program Files\Microsoft Visual Studio\2022\Community • Visual Studio Community 2022 version 17.11.35327.3 • Windows 10 SDK version 10.0.22621.0

[!] Android Studio (not installed) • Android Studio not found; download from https://developer.android.com/studio/index.html (or visit https://flutter.dev/to/windows-android-setup for detailed instructions).

[√] IntelliJ IDEA Community Edition (version 2024.2) • IntelliJ at C:\Users\vitoway\AppData\Local\JetBrains\IntelliJ IDEA Community Edition 2024.2.0.1 • Flutter plugin can be installed from: https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: https://plugins.jetbrains.com/plugin/6351-dart

[√] VS Code (version 1.94.2) • VS Code at C:\Users\vitoway\AppData\Local\Programs\Microsoft VS Code • Flutter extension version 3.98.0

[√] Connected device (3 available) • Windows (desktop) • windows • windows-x64 • Microsoft Windows [版本 10.0.22631.4317] • Chrome (web) • chrome • web-javascript • Google Chrome 129.0.6668.101 • Edge (web) • edge • web-javascript • Microsoft Edge 129.0.2792.89

[√] Network resources • All expected network resources are available.

! Doctor found issues in 2 categories. exit code 0

River-Zproject avatar Oct 17 '24 01:10 River-Zproject

Since @fufesou told me that this problem seemed to be introduced from version 3.22, I checked the version commit record. I found that https://github.com/flutter/engine/blob/1596c6bd884f1a3e697ae72e2aa602ce1d7ca8af/shell/platform/windows/egl/context.cc#L20 may be the place that caused the crash. Could you please help confirm it? @loic-sharma

The stack that caused the crash:

flutter_windows.dll!rx::StateManager11::onMakeCurrent(const gl::Context * context) 行 1805 C++ flutter_windows.dll!rx::Context11::onMakeCurrent(const gl::Context * context) 行 892 C++ flutter_windows.dll!gl::Context::makeCurrent(egl::Display * display, egl::Surface * drawSurface, egl::Surface * readSurface) 行 1023 C++ flutter_windows.dll!egl::Display::makeCurrent(egl::Thread * thread, gl::Context * previousContext, egl::Surface * drawSurface, egl::Surface * readSurface, gl::Context * context) 行 1722 C++ flutter_windows.dll!egl::Display::destroyContext(egl::Thread * thread, gl::Context * context) 行 1902 C++ flutter_windows.dll!egl::DestroyContext(egl::Thread * thread, egl::Display * display, gl::ContextID contextID) 行 292 C++ flutter_windows.dll!EGL_DestroyContext(void * dpy, void * ctx) 行 252 C++ flutter_windows.dll!eglDestroyContext(void * dpy, void * ctx) 行 117 C++ flutter_windows.dll!flutter::egl::Context::~Context() 行 20 C++ flutter_windows.dll!std::default_deleteflutter::egl::Context::operator()(flutter::egl::Context * _Ptr) 行 3302 C++ flutter_windows.dll!std::unique_ptr<flutter::egl::Context,std::default_deleteflutter::egl::Context>::reset(flutter::egl::Context * _Ptr) 行 3449 C++ flutter_windows.dll!flutter::egl::Manager::CleanUp() 行 222 C++ flutter_windows.dll!flutter::egl::Manager::~Manager() 行 45 C++ flutter_windows.dll!flutter::egl::Manager::~Manager() 行 44 C++ flutter_windows.dll!std::default_deleteflutter::egl::Manager::operator()(flutter::egl::Manager * _Ptr) 行 3303 C++ flutter_windows.dll!std::unique_ptr<flutter::egl::Manager,std::default_deleteflutter::egl::Manager>::~unique_ptr() 行 3414 C++ flutter_windows.dll!flutter::FlutterWindowsEngine::~FlutterWindowsEngine() 行 232 C++ flutter_windows.dll!flutter::FlutterWindowsEngine::~FlutterWindowsEngine() 行 229 C++ flutter_windows.dll!std::default_deleteflutter::FlutterWindowsEngine::operator()(flutter::FlutterWindowsEngine * _Ptr) 行 3303 C++ flutter_windows.dll!std::unique_ptr<flutter::FlutterWindowsEngine,std::default_deleteflutter::FlutterWindowsEngine>::reset(flutter::FlutterWindowsEngine * _Ptr) 行 3449 C++ flutter_windows.dll!flutter::FlutterWindowsViewController::Destroy() 行 28 C++ flutter_windows.dll!FlutterDesktopViewControllerDestroy(FlutterDesktopViewController * ref) 行 138 C++ desktop_multi_window_example.exe!flutter::FlutterViewController::~FlutterViewController() 行 28 C++ desktop_multi_window_example.exe!flutter::FlutterViewController::`scalar deleting destructor'(unsigned int) C++ desktop_multi_window_example.exe!std::default_deleteflutter::FlutterViewController::operator()(flutter::FlutterViewController * _Ptr) 行 3302 C++ desktop_multi_window_example.exe!std::unique_ptr<flutter::FlutterViewController,std::default_deleteflutter::FlutterViewController>::reset(flutter::FlutterViewController * _Ptr) 行 3447 C++ desktop_multi_window_example.exe!std::unique_ptr<flutter::FlutterViewController,std::default_deleteflutter::FlutterViewController>::operator=(void * formal) 行 3349 C++ desktop_multi_window_example.exe!FlutterWindow::OnDestroy() 行 45 C++ desktop_multi_window_example.exe!Win32Window::Destroy() 行 197 C++ desktop_multi_window_example.exe!Win32Window::MessageHandler(HWND * hwnd, const unsigned int message, const unsigned __int64 wparam, const int64 lparam) 行 160 C++ desktop_multi_window_example.exe!FlutterWindow::MessageHandler(HWND * hwnd, const unsigned int message, const unsigned __int64 wparam, const int64 lparam) 行 71 C++ desktop_multi_window_example.exe!Win32Window::WndProc(HWND * const window, const unsigned int message, const unsigned __int64 wparam, const int64 lparam) 行 146 C++ user32.dll!00007ffb64a383f1() 未知 user32.dll!00007ffb64a380ac() 未知 user32.dll!00007ffb64a431dd() 未知 ntdll.dll!KiUserCallbackDispatcherContinue() 未知 win32u.dll!00007ffb63621554() 未知 user32.dll!00007ffb64a36805() 未知 user32.dll!00007ffb64a36469() 未知 desktop_multi_window_example.exe!Win32Window::MessageHandler(HWND * hwnd, const unsigned int message, const unsigned __int64 wparam, const int64 lparam) 行 194 C++ desktop_multi_window_example.exe!FlutterWindow::MessageHandler(HWND * hwnd, const unsigned int message, const unsigned __int64 wparam, const int64 lparam) 行 71 C++ desktop_multi_window_example.exe!Win32Window::WndProc(HWND * const window, const unsigned int message, const unsigned int64 wparam, const int64 lparam) 行 146 C++ user32.dll!00007ffb64a383f1() 未知 user32.dll!00007ffb64a37eb1() 未知 desktop_multi_window_example.exe!wWinMain(HINSTANCE * instance, HINSTANCE * prev, wchar_t * command_line, int show_command) 行 38 C++ desktop_multi_window_example.exe!invoke_main() 行 123 C++ desktop_multi_window_example.exe!__scrt_common_main_seh() 行 288 C++ desktop_multi_window_example.exe!__scrt_common_main() 行 331 C++ desktop_multi_window_example.exe!wWinMainCRTStartup(void * __formal) 行 17 C++ kernel32.dll!00007ffb63b2257d() 未知 ntdll.dll!RtlUserThreadStart() 未知

Maybe the same cause #137973 (comment)

I suspect the proper fix is to make the FlutterViewController post a task to the raster thread to set the swap interval. This would ensure that the platform thread never makes the EGL context current during a rendering operation. I'll take a look at this.

@loic-sharma Could you please confirm?

This looks like a separate bug to me.

@fufesou Does this crash happen when you close the window? Perhaps there's a bug when the UI thread is closing a window and the raster thread is attempting to present to that same window. FYI, multi-window has been deprioritized so I likely won't have the bandwidth to investigate this myself. I'm happy to answer any questions and help you debug though! Sorry :(

When window A is being created, window B is being closed, and the closing releases the context causing the rendering of window A to crash.

River-Zproject avatar Oct 22 '24 06:10 River-Zproject

From the call stack you shared, it looks like destroying the window also destroyed the engine (flutter_windows.dll!flutter::FlutterWindowsEngine::~FlutterWindowsEngine() call stack line).

Are you using the current Flutter Windows C++ APIs? These don't support multi-window properly. If you want to use multi-window on Windows, you either need to build a custom engine with this patch, or, you need to hack your app to link against APIs that aren't public yet (example). Both approaches are highly experimental and might break at any time.

loic-sharma avatar Oct 22 '24 21:10 loic-sharma

Triage note: we should update FlutterDesktopViewControllerCreate to fail (return nullptr) if the engine is already owned by another view controller.

https://github.com/flutter/engine/blob/70671baabbba922296e7e737e317753666678093/shell/platform/windows/public/flutter_windows.h#L75-L94

loic-sharma avatar Oct 28 '24 17:10 loic-sharma

Hi @loic-sharma , Sorry for replying so late, because I just got to know Flutter for a few days. I don't even know what Flutter Windows C++ APIs are. I just know that we are using this plugin desktop_multi_window . This plugin does not build a multi-view application as Flutter envisions. We build multi-view applications through multi-engines, and each window is like an independent Flutter application. When we closed the window, the Flutter engine was indeed closed.

These days, I debugged the process of shutting down the Flutter engine. On 3.19.6 and 3.24.3, I found a difference, which led to the stack mentioned above.

On 3.19.6, because there is no multi-view related code, there is no EGL context switching operation before shutting down the Flutter engine. Therefore, the mRefCount of the EGL context does not change and remains at 1 until the Flutter engine is shut down. This ensures that egl::Display::destroyContext will not go to rx::StateManager11::onMakeCurrent.

egl::Display::destroyContext https://github.com/google/angle/blob/0a372f29da7c5fa2d4412feff991daca69d0164d/src/libANGLE/Display.cpp#L1872

On 3.24.3, before shutting down the Flutter engine, there are three mRefCount-related operations:

  1. egl::Surface::Destroy() causes mRefCount to decrease by one https://github.com/flutter/engine/blob/5fa7a123b14a184272ce0165cdcef683daf73e45/shell/platform/windows/egl/surface.cc#L26

  2. flutter::Rasterizer::Teardown() causes mRefCount to increase by one and then decrease by one increase code https://github.com/flutter/engine/blob/052b970eb443e79fb45790fc1d956d1ed2f7d258/shell/common/rasterizer.cc#L122 decrease code https://github.com/flutter/engine/blob/052b970eb443e79fb45790fc1d956d1ed2f7d258/shell/common/rasterizer.cc#L131

I personally guess that the behavior of egl::Surface::Destroy() causes the difference between versions 3.24.3 and 3.19.6. I have tried to let flutter::Rasterizer::Teardown() not decrease mRefCount by one, which can solve the crash problem, but I don’t think this is the right way to modify it.

I hope you can give some suggestions on this issue. Thank you very much!

River-Zproject avatar Oct 29 '24 09:10 River-Zproject

I finally found what was actually causing the difference, where ClearContext was changed to ClearCurrent. I tried reverting it back and that worked fine, resolving the crash.

https://github.com/flutter/engine/pull/49954/files#diff-6fb5dee70153936fa0a06844e04a5bddbf779750e4670292b8f77eaae36c52bcR68

Could you please help confirm it ? @loic-sharma

River-Zproject avatar Oct 31 '24 02:10 River-Zproject

Thanks for the investigation!

Before this change, Windows's clear_current_callback unbinds the surfaces but binds the render context (here)

After this change, Windows's clear_current_callback unbinds the surfaces and context (here).

This wasn't an intentional change, but off-hand that seems like it should be fine. I've reached out to engine folks to see if this was an incorrect change.

@hbatagelo is this something you could address as part of Canonical's multi-window effort?

loic-sharma avatar Nov 02 '24 00:11 loic-sharma

Notes from engine team discussion:

  1. The new clear_current_callback looks correct

  2. However, make_current_callback should make a surface current. Today, Windows's callback makes the context current without a surface. This will return EGL_BAD_MATCH if the device does not support surfaceless contexts.

    Only half of devices support surfaceless contexts (the EGL_KHR_surfaceless_context extension).

    Impeller creates a throwaway surface to avoid depending on this extension. Windows should consider this approach if it needs surfaceless contexts.

This is likely a separate problem from this issue.

--

I wonder if the root cause is a similar problem to the one described in this issue: https://github.com/flutter/flutter/issues/138893#issuecomment-1852109576

loic-sharma avatar Nov 05 '24 22:11 loic-sharma

Yes, that should be the root cause. This confirms some of my guesses.

After rolling back clear_current_callback, I found that the memory leak phenomenon in 3.24 is consistent with 3.19. The phenomenon is that there is about 60M memory leak when creating and closing 100 windows. This situation is acceptable to us because so many windows will not be created in actual use.

In fact, another resource context has the same memory leak problem and the same reason. https://github.com/flutter/engine/blob/382fd1ea143afd57c1191500d75ac23984e939cf/shell/platform/windows/egl/manager.cc#L223

Regarding the memory leak problem, I have actually been thinking about how to safely destroy the context.

River-Zproject avatar Nov 06 '24 06:11 River-Zproject

@hbatagelo is this something you could address as part of Canonical's multi-window effort?

Thanks for making me aware of this issue. I'll keep an eye on it, as well as on https://github.com/flutter/flutter/issues/138893. Maybe we can gain better insight by observing how the leaks impact the multi-window scenario, both with a single engine and with multiple engines.

hbatagelo avatar Nov 06 '24 14:11 hbatagelo