From 3ca033ce4e1e592c0989f908e5566581d236531c Mon Sep 17 00:00:00 2001 From: Mac Date: Mon, 26 Oct 2026 11:40:04 +0100 Subject: [PATCH 1/2] [vector_graphics] Dispose pictures decoded after their VectorGraphic is removed --- packages/vector_graphics/CHANGELOG.md | 5 + .../lib/src/vector_graphics.dart | 11 +- packages/vector_graphics/pubspec.yaml | 2 +- .../vector_graphics/test/caching_test.dart | 122 +++++++++++++++++- 4 files changed, 133 insertions(+), 7 deletions(-) diff --git a/packages/vector_graphics/CHANGELOG.md b/packages/vector_graphics/CHANGELOG.md index 8c2dce6bb06b..b9fc41f50d5e 100644 --- a/packages/vector_graphics/CHANGELOG.md +++ b/packages/vector_graphics/CHANGELOG.md @@ -1,3 +1,8 @@ +## 1.2.5 + +* Fixes a leak of the decoded picture when a `VectorGraphic` is removed before + loading finishes. + ## 1.2.4 * Fixes an unhandled exception reported for every asynchronous loading failure, diff --git a/packages/vector_graphics/lib/src/vector_graphics.dart b/packages/vector_graphics/lib/src/vector_graphics.dart index 23f36c44a5a9..63064f55f5ea 100644 --- a/packages/vector_graphics/lib/src/vector_graphics.dart +++ b/packages/vector_graphics/lib/src/vector_graphics.dart @@ -421,18 +421,19 @@ class _VectorGraphicWidgetState extends State { try { final _PictureData data = await _loadPicture(context, key, loader); data.count += 1; + if (data.count == 1) { + _livePictureCache[key] = data; + } // The widget may have changed, requesting a new vector graphic before // this operation could complete. if (!mounted || loader != widget.loader) { - _maybeReleasePicture(data); + // Other widgets waiting on the same load resume after this one and may + // still use the picture, so only release it once they have. + scheduleMicrotask(() => _maybeReleasePicture(data)); return; } - if (data.count == 1) { - _livePictureCache[key] = data; - } - setState(() { _maybeReleasePicture(_pictureData); _pictureData = data; diff --git a/packages/vector_graphics/pubspec.yaml b/packages/vector_graphics/pubspec.yaml index 22e8eed23c79..73c5831f219f 100644 --- a/packages/vector_graphics/pubspec.yaml +++ b/packages/vector_graphics/pubspec.yaml @@ -2,7 +2,7 @@ name: vector_graphics description: A vector graphics rendering package for Flutter using a binary encoding. repository: https://github.com/flutter/packages/tree/main/packages/vector_graphics issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+vector_graphics%22 -version: 1.2.4 +version: 1.2.5 environment: sdk: ^3.11.0 diff --git a/packages/vector_graphics/test/caching_test.dart b/packages/vector_graphics/test/caching_test.dart index 33b9a81dc360..fc013e2252e6 100644 --- a/packages/vector_graphics/test/caching_test.dart +++ b/packages/vector_graphics/test/caching_test.dart @@ -3,10 +3,12 @@ // found in the LICENSE file. import 'dart:async'; -import 'dart:typed_data'; +import 'dart:ui' as ui; +import 'package:flutter/foundation.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:vector_graphics/src/render_vector_graphic.dart'; import 'package:vector_graphics/src/vector_graphics.dart'; import 'package:vector_graphics_codec/vector_graphics_codec.dart'; @@ -249,6 +251,110 @@ void main() { expect(testBundle.loadKeys, ['bar.svg', 'foo.svg', 'foo.svg']); }); + + group('Pictures decoded after their VectorGraphic is removed', () { + // Collects the native pictures created and disposed during a test. + (Set, Set) trackPictures() { + final created = {}; + final disposed = {}; + void listener(ObjectEvent event) { + if (event.object.runtimeType.toString() != '_NativePicture') { + return; + } + if (event is ObjectCreated) { + created.add(event.object); + } else if (event is ObjectDisposed) { + disposed.add(event.object); + } + } + + FlutterMemoryAllocations.instance.addListener(listener); + addTearDown(() => FlutterMemoryAllocations.instance.removeListener(listener)); + return (created, disposed); + } + + testWidgets('are disposed', (WidgetTester tester) async { + final loader = CompleterBytesLoader(); + final (Set created, Set disposed) = trackPictures(); + + await tester.pumpWidget(VectorGraphic(loader: loader)); + await tester.pumpWidget(const SizedBox()); + loader.complete(); + await tester.pumpAndSettle(); + + expect(created, hasLength(1)); + expect(disposed, containsAll(created)); + }); + + testWidgets('are disposed once when every widget sharing the load is removed', ( + WidgetTester tester, + ) async { + final loader = CompleterBytesLoader(); + final (Set created, Set disposed) = trackPictures(); + + await tester.pumpWidget( + Column( + children: [ + VectorGraphic(loader: loader), + VectorGraphic(loader: loader), + ], + ), + ); + await tester.pumpWidget(const SizedBox()); + loader.complete(); + await tester.pumpAndSettle(); + + expect(tester.takeException(), isNull); + expect(created, hasLength(1)); + expect(disposed, containsAll(created)); + }); + + testWidgets('stay alive while another widget sharing the load shows them', ( + WidgetTester tester, + ) async { + final loader = CompleterBytesLoader(); + + await tester.pumpWidget( + Column( + children: [ + VectorGraphic(key: GlobalKey(), loader: loader), + VectorGraphic(loader: loader), + ], + ), + ); + await tester.pumpWidget(Column(children: [VectorGraphic(loader: loader)])); + loader.complete(); + await tester.pumpAndSettle(); + + expect(tester.takeException(), isNull); + final ui.Picture picture = tester.allRenderObjects + .whereType() + .single + .pictureInfo + .picture; + expect(picture.debugDisposed, isFalse); + + await tester.pumpWidget(const SizedBox()); + expect(picture.debugDisposed, isTrue); + }); + + testWidgets('are disposed when the loader changes before decoding finishes', ( + WidgetTester tester, + ) async { + final first = CompleterBytesLoader(); + final second = CompleterBytesLoader(); + + await tester.pumpWidget(VectorGraphic(loader: first)); + await tester.pumpWidget(VectorGraphic(loader: second)); + final (Set created, Set disposed) = trackPictures(); + first.complete(); + await tester.pumpAndSettle(); + + expect(tester.takeException(), isNull); + expect(created, hasLength(1)); + expect(disposed, containsAll(created)); + }); + }); } class TestAssetBundle extends Fake implements AssetBundle { @@ -296,3 +402,17 @@ class TestWidgetsLocalizations extends DefaultWidgetsLocalizations { @override TextDirection get textDirection => TextDirection.ltr; } + +class CompleterBytesLoader extends BytesLoader { + final Completer _completer = Completer(); + + /// Finishes loading with a small, empty vector graphic. + void complete() { + final buffer = VectorGraphicsBuffer(); + codec.writeSize(buffer, 10, 10); + _completer.complete(buffer.done()); + } + + @override + Future loadBytes(BuildContext? context) => _completer.future; +} From 19b4a95c8338c4382e6eca7e81a8749b6a4f4b39 Mon Sep 17 00:00:00 2001 From: Mac Date: Wed, 28 Oct 2026 16:08:10 +0100 Subject: [PATCH 2/2] Make picture tests work on web and clarify the release comment --- .../lib/src/vector_graphics.dart | 5 +-- .../vector_graphics/test/caching_test.dart | 31 ++++++++----------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/packages/vector_graphics/lib/src/vector_graphics.dart b/packages/vector_graphics/lib/src/vector_graphics.dart index 63064f55f5ea..1c0646b2eedf 100644 --- a/packages/vector_graphics/lib/src/vector_graphics.dart +++ b/packages/vector_graphics/lib/src/vector_graphics.dart @@ -428,8 +428,9 @@ class _VectorGraphicWidgetState extends State { // The widget may have changed, requesting a new vector graphic before // this operation could complete. if (!mounted || loader != widget.loader) { - // Other widgets waiting on the same load resume after this one and may - // still use the picture, so only release it once they have. + // Other widgets awaiting the same load resume after this one, all in the + // same microtask. Defer the release to a later microtask so they can claim + // the picture first; otherwise it could be disposed while they still need it. scheduleMicrotask(() => _maybeReleasePicture(data)); return; } diff --git a/packages/vector_graphics/test/caching_test.dart b/packages/vector_graphics/test/caching_test.dart index fc013e2252e6..81244306fe9c 100644 --- a/packages/vector_graphics/test/caching_test.dart +++ b/packages/vector_graphics/test/caching_test.dart @@ -253,29 +253,24 @@ void main() { }); group('Pictures decoded after their VectorGraphic is removed', () { - // Collects the native pictures created and disposed during a test. - (Set, Set) trackPictures() { - final created = {}; - final disposed = {}; + // Collects the pictures created during a test. + Set trackPictures() { + final created = {}; void listener(ObjectEvent event) { - if (event.object.runtimeType.toString() != '_NativePicture') { - return; - } - if (event is ObjectCreated) { - created.add(event.object); - } else if (event is ObjectDisposed) { - disposed.add(event.object); + final Object object = event.object; + if (event is ObjectCreated && object is ui.Picture) { + created.add(object); } } FlutterMemoryAllocations.instance.addListener(listener); addTearDown(() => FlutterMemoryAllocations.instance.removeListener(listener)); - return (created, disposed); + return created; } testWidgets('are disposed', (WidgetTester tester) async { final loader = CompleterBytesLoader(); - final (Set created, Set disposed) = trackPictures(); + final Set created = trackPictures(); await tester.pumpWidget(VectorGraphic(loader: loader)); await tester.pumpWidget(const SizedBox()); @@ -283,14 +278,14 @@ void main() { await tester.pumpAndSettle(); expect(created, hasLength(1)); - expect(disposed, containsAll(created)); + expect(created.single.debugDisposed, isTrue); }); testWidgets('are disposed once when every widget sharing the load is removed', ( WidgetTester tester, ) async { final loader = CompleterBytesLoader(); - final (Set created, Set disposed) = trackPictures(); + final Set created = trackPictures(); await tester.pumpWidget( Column( @@ -306,7 +301,7 @@ void main() { expect(tester.takeException(), isNull); expect(created, hasLength(1)); - expect(disposed, containsAll(created)); + expect(created.single.debugDisposed, isTrue); }); testWidgets('stay alive while another widget sharing the load shows them', ( @@ -344,15 +339,15 @@ void main() { final first = CompleterBytesLoader(); final second = CompleterBytesLoader(); + final Set created = trackPictures(); await tester.pumpWidget(VectorGraphic(loader: first)); await tester.pumpWidget(VectorGraphic(loader: second)); - final (Set created, Set disposed) = trackPictures(); first.complete(); await tester.pumpAndSettle(); expect(tester.takeException(), isNull); expect(created, hasLength(1)); - expect(disposed, containsAll(created)); + expect(created.single.debugDisposed, isTrue); }); }); }