Skip to content

Commit

Permalink
Fix an issue that clearing the image cache may cause resource leaks (…
Browse files Browse the repository at this point in the history
…#104527)
  • Loading branch information
yeatse committed May 24, 2022
1 parent a56c5e5 commit b5adbee
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 12 deletions.
23 changes: 11 additions & 12 deletions packages/flutter/lib/src/painting/image_cache.dart
Expand Up @@ -397,16 +397,16 @@ class ImageCache {
if (!kReleaseMode) {
listenerTask = TimelineTask(parent: timelineTask)..start('listener');
}
// If we're doing tracing, we need to make sure that we don't try to finish
// the trace entry multiple times if we get re-entrant calls from a multi-
// frame provider here.
// A multi-frame provider may call the listener more than once. We need do make
// sure that some cleanup works won't run multiple times, such as finishing the
// tracing task or removing the listeners
bool listenedOnce = false;

// We shouldn't use the _pendingImages map if the cache is disabled, but we
// will have to listen to the image at least once so we don't leak it in
// the live image tracking.
// If the cache is disabled, this variable will be set.
_PendingImage? untrackedPendingImage;
final bool trackPendingImage = maximumSize > 0 && maximumSizeBytes > 0;
late _PendingImage pendingImage;
void listener(ImageInfo? info, bool syncCall) {
int? sizeBytes;
if (info != null) {
Expand All @@ -421,14 +421,14 @@ class ImageCache {
_trackLiveImage(key, result, sizeBytes);

// Only touch if the cache was enabled when resolve was initially called.
if (untrackedPendingImage == null) {
if (trackPendingImage) {
_touch(key, image, listenerTask);
} else {
image.dispose();
}

final _PendingImage? pendingImage = untrackedPendingImage ?? _pendingImages.remove(key);
if (pendingImage != null) {
_pendingImages.remove(key);
if (!listenedOnce) {
pendingImage.removeListener();
}
if (!kReleaseMode && !listenedOnce) {
Expand All @@ -445,10 +445,9 @@ class ImageCache {
}

final ImageStreamListener streamListener = ImageStreamListener(listener);
if (maximumSize > 0 && maximumSizeBytes > 0) {
_pendingImages[key] = _PendingImage(result, streamListener);
} else {
untrackedPendingImage = _PendingImage(result, streamListener);
pendingImage = _PendingImage(result, streamListener);
if (trackPendingImage) {
_pendingImages[key] = pendingImage;
}
// Listener is removed in [_PendingImage.removeListener].
result.addListener(streamListener);
Expand Down
27 changes: 27 additions & 0 deletions packages/flutter/test/painting/image_cache_test.dart
Expand Up @@ -332,6 +332,33 @@ void main() {
expect(imageCache.liveImageCount, 0);
});

test('Clearing image cache does not leak live images', () async {
imageCache.maximumSize = 1;

final ui.Image testImage1 = await createTestImage(width: 8, height: 8);
final ui.Image testImage2 = await createTestImage(width: 10, height: 10);

final TestImageStreamCompleter completer1 = TestImageStreamCompleter();
final TestImageStreamCompleter completer2 = TestImageStreamCompleter()..testSetImage(testImage2);

imageCache.putIfAbsent(testImage1, () => completer1);
expect(imageCache.statusForKey(testImage1).pending, true);
expect(imageCache.statusForKey(testImage1).live, true);

imageCache.clear();
expect(imageCache.statusForKey(testImage1).pending, false);
expect(imageCache.statusForKey(testImage1).live, true);

completer1.testSetImage(testImage1);
expect(imageCache.statusForKey(testImage1).keepAlive, true);
expect(imageCache.statusForKey(testImage1).live, false);

imageCache.putIfAbsent(testImage2, () => completer2);
expect(imageCache.statusForKey(testImage1).tracked, false); // evicted
expect(imageCache.statusForKey(testImage2).tracked, true);
});


test('Evicting a pending image clears the live image by default', () async {
final ui.Image testImage = await createTestImage(width: 8, height: 8);

Expand Down

0 comments on commit b5adbee

Please sign in to comment.