[stable] [Impeller] Fix app crash on some Android devices due to VK cache corruption #176520
Conversation
…tter#173014) PipelineCacheDataPersist runs on a worker thread pool. The raster thread may be executing rendering operations while PipelineCacheDataPersist is reading the state of the pipeline cache. PipelineCacheDataPersist calls vkGetPipelineCacheData to get the size required for the cache data buffer and then calls it again to fill the buffer. If the cache state changed between those two calls, then the count of bytes written may be less than the size returned by the first call. In that case, PipelineCacheDataPersist should only write the portion of the buffer that was filled. This PR also adds a mutex to PipelineCacheVK::PersistCacheToDisk. PipelineLibraryVK could queue multiple PersistCacheToDisk tasks to different worker pool threads. These tasks should not run concurrently. See flutter#172624
|
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. |
There was a problem hiding this comment.
Code Review
This pull request cherry-picks a critical fix for a crash on some Android devices caused by a corrupted Vulkan pipeline cache. The changes correctly address the issue by ensuring the cache file header reflects the actual size of the written cache data, even when the driver reports a smaller size than initially queried. A new unit test has been added to verify this fix using a mock Vulkan implementation that simulates the problematic driver behavior. Additionally, a potential race condition in persisting the cache to disk has been resolved by introducing a mutex, improving thread safety. The changes are well-implemented and effectively resolve the reported crash.
|
Triage: The commit author has verified that this is low risk and the only contents of this patch is the revert. We need to verify if the release window is still open. Pinging the release hotline. |
|
@chinmaygarde or @jason-simmons can you approve this pr if you are comfortable with it landing? |
d291363
into
flutter:flutter-3.35-candidate.0
Updates `CHANGELOG` to include the two cherry picks in the 3.35.6 hotfix stable release: #176520 #176359 **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.
…ue to VK cache corruption (flutter/flutter#176520)
…ue to VK cache corruption (flutter/flutter#176520)
…ue to VK cache corruption (flutter/flutter#176520)
Stable Cherry Pick
Cherry-picks #173014 to stable to fix unhandled crash on Android Snapdragon 845 devices due to a corrupted cache, preventing apps from starting.
Fixes #172624
Impacted Users: All users with Android Snapdragon 845
Impact Description: the app can crash on start because of a corrupted cache.
Workaround: None
Risk: Low
Test Coverage: A test is included that checks succesful reads of an incomplete vk cache
Validation Steps: #172624 (comment)
This crash has unfortunately prevented our app from using Flutter 3.35 and even 3.32, missing out a whole new set of fixes from these last Flutter releases for quite some time now.
cc @jason-simmons (PR author) @chinmaygarde (PR reviewer) in case there is something I could have missed with the CP request and description