[CP-beta] [Impeller] Wait for the Vulkan device to become idle before destroying Vulkan objects in the AHBSwapchainImplVK destructor (#187477)#188588
Conversation
…g Vulkan objects in the AHBSwapchainImplVK destructor (flutter#187477) 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. |
|
@jason-simmons, re-opened #188519 to target the new beta branch 3.46 |
There was a problem hiding this comment.
Code Review
This pull request updates AHBSwapchainImplVK to call WaitIdle() in its destructor, ensuring the Vulkan device is idle before destroying objects. It also adds a unit test to verify this behavior and mocks vkDeviceWaitIdle for testing. The review feedback suggests upgrading the log level from INFO to ERROR when vkDeviceWaitIdle fails, as a failure indicates a critical device issue.
| if (result != vk::Result::eSuccess) { | ||
| FML_LOG(INFO) << "Device waitIdle failed: " << vk::to_string(result); | ||
| } |
There was a problem hiding this comment.
A failure in vkDeviceWaitIdle (such as VK_ERROR_DEVICE_LOST) is a serious error that indicates a critical issue with the Vulkan device. Logging this as FML_LOG(INFO) makes it highly likely to be overlooked in production logs. It should be logged as FML_LOG(ERROR) to ensure visibility during debugging and crash analysis.
| if (result != vk::Result::eSuccess) { | |
| FML_LOG(INFO) << "Device waitIdle failed: " << vk::to_string(result); | |
| } | |
| if (result != vk::Result::eSuccess) { | |
| FML_LOG(ERROR) << "Device waitIdle failed: " << vk::to_string(result); | |
| } |
|
autosubmit label was removed for flutter/flutter/188588, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label. |
a24b1ea
into
flutter:flutter-3.46-candidate.0
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.