Repository navigation
[CP-stable][Impeller] Wait for the Vulkan device to become idle before destroying Vulkan objects in the AHBSwapchainImplVK destructor - #188531
Conversation
…e destroying Vulkan objects in the AHBSwapchainImplVK destructor AHBSwapchainImplVK holds Vulkan objects such as fences that may be used by operations submitted to the Vulkan device. The AHBSwapchainImplVK destructor must wait until the device is no longer using these objects before destroying them. Fixes flutter#187237
|
This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter. Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed. |
github-actions
Bot
added
engine
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors SurfaceControl into an abstract interface with a concrete SurfaceControlImpl implementation, updates AHBSwapchainVK to accept a shared pointer to SurfaceControl, and ensures the Vulkan device is idle before destroying AHBSwapchainImplVK. The review feedback recommends using the override specifier for the destructor in SurfaceControlImpl, preferring std::make_unique over explicit new when instantiating SurfaceControl, and optimizing std::shared_ptr parameters by passing them by value and using std::move to avoid unnecessary reference count overhead.
|
The only part that is required to fix the bug is the change to call (58709f2#diff-953ce211fc3866f71b5594b6697f6973b9d9a02309972d4e06960bddd2b9c450R105) The rest of this PR is a refactoring that is needed to support the unit test (taken from 2c9accd) |
|
@jason-simmons the tense/direction is wrong on your changelog entry we use the state of the code prior to the the cherry pick for descriptions. see: https://github.com/flutter/flutter/blob/main/docs/releases/Hotfix-Documentation-Best-Practices.md |
reidbaker
added
the
autosubmit
97f70d2
into
flutter:flutter-3.44-candidate.0
…e destroying Vulkan objects in the AHBSwapchainImplVK destructor (flutter#188531) ### Issue Link: flutter#187237 ### Impact Description: This can cause a crash on Impeller/Vulkan during app lifecycle events (shutdown/rotation/etc.) when using HCPP/AHBSwapchain. ### Changelog Description: [flutter/187237] Fixes a crash that can happen during app shutdown or rotation on some Android devices using Impeller/Vulkan with HCPP. ### Workaround: Disable Impeller or use the Impeller GLES back end ### Risk: What is the risk level of this cherry-pick? ### Test Coverage: Are you confident that your fix is well-tested by automated tests? ### Validation Steps: Run the `AndroidAHBSwapchainTest.AHBSwapchainDtorCallsWaitIdle` unit test. Or try reproducing the crash in the flutter#187237 example app and verifying that it is fixed.
3 participants
Issue Link:
#187237
Impact Description:
This can cause a crash on Impeller/Vulkan during app lifecycle events (shutdown/rotation/etc.) when using HCPP/AHBSwapchain.
Changelog Description:
[flutter/187237] Fixes a crash that can happen during app shutdown or rotation on some Android devices using Impeller/Vulkan with HCPP.
Workaround:
Disable Impeller or use the Impeller GLES back end
Risk:
What is the risk level of this cherry-pick?
Test Coverage:
Are you confident that your fix is well-tested by automated tests?
Validation Steps:
Run the
AndroidAHBSwapchainTest.AHBSwapchainDtorCallsWaitIdleunit test. Or try reproducing the crash in the #187237 example app and verifying that it is fixed.