-
Notifications
You must be signed in to change notification settings - Fork 6k
[web] Don't close image source too early #57200
Conversation
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for the quick fix!
A `CkImage` instance holds a reference to `ImageSource?`. When that `CkImage` gets cloned, the `ImageSource` instance becomes shared between the original `CkImage` and its new clone. Then when one of the `CkImage`s gets disposed of, it closes the shared `ImageSource` leaving other live `CkImage`s holding on to a closed `ImageSource`. The quick solution to this is to have a ref count on the `ImageSource` to count how many `CkImage`s are referencing it. The `ImageSource` will only be closed if its ref count reaches 0. Fixes flutter/flutter#160199 Fixes flutter/flutter#158093
…160385) flutter/engine@0dfbd04...61616ac 2024-12-16 [email protected] Migrate FlBinaryMessenger using embedder API instead of mock engine. (flutter/engine#57214) 2024-12-16 [email protected] Flutter-GPU.md: Fix spelling (flutter/engine#57216) 2024-12-16 [email protected] [web] Don't close image source too early (flutter/engine#57200) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
A `CkImage` instance holds a reference to `ImageSource?`. When that `CkImage` gets cloned, the `ImageSource` instance becomes shared between the original `CkImage` and its new clone. Then when one of the `CkImage`s gets disposed of, it closes the shared `ImageSource` leaving other live `CkImage`s holding on to a closed `ImageSource`. The quick solution to this is to have a ref count on the `ImageSource` to count how many `CkImage`s are referencing it. The `ImageSource` will only be closed if its ref count reaches 0. Fixes flutter#160199 Fixes flutter#158093
A
CkImage
instance holds a reference toImageSource?
. When thatCkImage
gets cloned, theImageSource
instance becomes shared between the originalCkImage
and its new clone. Then when one of theCkImage
s gets disposed of, it closes the sharedImageSource
leaving other liveCkImage
s holding on to a closedImageSource
.The quick solution to this is to have a ref count on the
ImageSource
to count how manyCkImage
s are referencing it. TheImageSource
will only be closed if its ref count reaches 0.Fixes flutter/flutter#160199
Fixes flutter/flutter#158093