From ad4dea37d77751b1fd2150099a74224bf4abd27b Mon Sep 17 00:00:00 2001 From: David Martos Date: Sat, 22 Oct 2022 09:34:33 +0200 Subject: [PATCH 1/3] Close chunk events stream controller when imageStream is disposed --- .../cached_network_image_provider.dart | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/cached_network_image/lib/src/image_provider/cached_network_image_provider.dart b/cached_network_image/lib/src/image_provider/cached_network_image_provider.dart index 039f2d9c..86192b48 100644 --- a/cached_network_image/lib/src/image_provider/cached_network_image_provider.dart +++ b/cached_network_image/lib/src/image_provider/cached_network_image_provider.dart @@ -77,7 +77,7 @@ class CachedNetworkImageProvider ImageStreamCompleter load( image_provider.CachedNetworkImageProvider key, DecoderCallback decode) { final chunkEvents = StreamController(); - return MultiImageStreamCompleter( + final imageStreamCompleter = MultiImageStreamCompleter( codec: _loadAsync(key, chunkEvents, decode), chunkEvents: chunkEvents.stream, scale: key.scale, @@ -89,6 +89,10 @@ class CachedNetworkImageProvider ); }, ); + + _handleChunkEventsClose(imageStreamCompleter, chunkEvents); + + return imageStreamCompleter; } @Deprecated( @@ -118,7 +122,7 @@ class CachedNetworkImageProvider ImageStreamCompleter loadBuffer(image_provider.CachedNetworkImageProvider key, DecoderBufferCallback decode) { final chunkEvents = StreamController(); - return MultiImageStreamCompleter( + final imageStreamCompleter = MultiImageStreamCompleter( codec: _loadBufferAsync(key, chunkEvents, decode), chunkEvents: chunkEvents.stream, scale: key.scale, @@ -130,6 +134,10 @@ class CachedNetworkImageProvider ); }, ); + + _handleChunkEventsClose(imageStreamCompleter, chunkEvents); + + return imageStreamCompleter; } Stream _loadBufferAsync( @@ -164,6 +172,21 @@ class CachedNetworkImageProvider return false; } + void _handleChunkEventsClose( + ImageStreamCompleter imageStreamCompleter, + StreamController chunkEvents, + ) { + // Make sure to close the chunk events controller after + // the image stream disposes. Otherwise reporting an image chunk + // event could fail beacause the ImageStream has been disposed. + // (https://github.com/Baseflow/flutter_cached_network_image/issues/785) + imageStreamCompleter.addOnLastListenerRemovedCallback(() { + if (!chunkEvents.isClosed) { + chunkEvents.close(); + } + }); + } + @override int get hashCode => Object.hash(cacheKey ?? url, scale, maxHeight, maxWidth); From ce6d67111c727c3a44849d67d1c730985d999c9e Mon Sep 17 00:00:00 2001 From: David Martos Date: Sat, 22 Oct 2022 10:47:14 +0200 Subject: [PATCH 2/3] Unsubscribe from chunkEvents --- .../cached_network_image_provider.dart | 27 +-------- .../multi_image_stream_completer.dart | 60 ++++++++++++++++++- .../test/image_stream_completer_test.dart | 31 ++++++++++ 3 files changed, 92 insertions(+), 26 deletions(-) diff --git a/cached_network_image/lib/src/image_provider/cached_network_image_provider.dart b/cached_network_image/lib/src/image_provider/cached_network_image_provider.dart index 86192b48..039f2d9c 100644 --- a/cached_network_image/lib/src/image_provider/cached_network_image_provider.dart +++ b/cached_network_image/lib/src/image_provider/cached_network_image_provider.dart @@ -77,7 +77,7 @@ class CachedNetworkImageProvider ImageStreamCompleter load( image_provider.CachedNetworkImageProvider key, DecoderCallback decode) { final chunkEvents = StreamController(); - final imageStreamCompleter = MultiImageStreamCompleter( + return MultiImageStreamCompleter( codec: _loadAsync(key, chunkEvents, decode), chunkEvents: chunkEvents.stream, scale: key.scale, @@ -89,10 +89,6 @@ class CachedNetworkImageProvider ); }, ); - - _handleChunkEventsClose(imageStreamCompleter, chunkEvents); - - return imageStreamCompleter; } @Deprecated( @@ -122,7 +118,7 @@ class CachedNetworkImageProvider ImageStreamCompleter loadBuffer(image_provider.CachedNetworkImageProvider key, DecoderBufferCallback decode) { final chunkEvents = StreamController(); - final imageStreamCompleter = MultiImageStreamCompleter( + return MultiImageStreamCompleter( codec: _loadBufferAsync(key, chunkEvents, decode), chunkEvents: chunkEvents.stream, scale: key.scale, @@ -134,10 +130,6 @@ class CachedNetworkImageProvider ); }, ); - - _handleChunkEventsClose(imageStreamCompleter, chunkEvents); - - return imageStreamCompleter; } Stream _loadBufferAsync( @@ -172,21 +164,6 @@ class CachedNetworkImageProvider return false; } - void _handleChunkEventsClose( - ImageStreamCompleter imageStreamCompleter, - StreamController chunkEvents, - ) { - // Make sure to close the chunk events controller after - // the image stream disposes. Otherwise reporting an image chunk - // event could fail beacause the ImageStream has been disposed. - // (https://github.com/Baseflow/flutter_cached_network_image/issues/785) - imageStreamCompleter.addOnLastListenerRemovedCallback(() { - if (!chunkEvents.isClosed) { - chunkEvents.close(); - } - }); - } - @override int get hashCode => Object.hash(cacheKey ?? url, scale, maxHeight, maxWidth); diff --git a/cached_network_image/lib/src/image_provider/multi_image_stream_completer.dart b/cached_network_image/lib/src/image_provider/multi_image_stream_completer.dart index 80c1dac5..1ceef802 100644 --- a/cached_network_image/lib/src/image_provider/multi_image_stream_completer.dart +++ b/cached_network_image/lib/src/image_provider/multi_image_stream_completer.dart @@ -38,7 +38,7 @@ class MultiImageStreamCompleter extends ImageStreamCompleter { ); }); if (chunkEvents != null) { - chunkEvents.listen( + _chunkSubscription = chunkEvents.listen( reportImageChunkEvent, onError: (dynamic error, StackTrace stack) { reportError( @@ -65,10 +65,17 @@ class MultiImageStreamCompleter extends ImageStreamCompleter { // How many frames have been emitted so far. int _framesEmitted = 0; Timer? _timer; + StreamSubscription? _chunkSubscription; // Used to guard against registering multiple _handleAppFrame callbacks for the same frame. bool _frameCallbackScheduled = false; + /// We must avoid disposing a completer if it has never had a listener, even + /// if all [keepAlive] handles get disposed. + bool __hadAtLeastOneListener = false; + + bool __disposed = false; + void _switchToNewCodec() { _framesEmitted = 0; _timer = null; @@ -159,6 +166,7 @@ class MultiImageStreamCompleter extends ImageStreamCompleter { @override void addListener(ImageStreamListener listener) { + __hadAtLeastOneListener = true; if (!hasListeners && _codec != null) _decodeNextFrameAndSchedule(); super.addListener(listener); } @@ -169,6 +177,56 @@ class MultiImageStreamCompleter extends ImageStreamCompleter { if (!hasListeners) { _timer?.cancel(); _timer = null; + __maybeDispose(); } } + + int __keepAliveHandles = 0; + + @override + ImageStreamCompleterHandle keepAlive() { + final delegateHandle = super.keepAlive(); + return _MultiImageStreamCompleterHandle(this, delegateHandle); + } + + void __maybeDispose() { + if (!__hadAtLeastOneListener || + __disposed || + hasListeners || + __keepAliveHandles != 0) { + return; + } + + __disposed = true; + + _chunkSubscription?.onData(null); + _chunkSubscription?.cancel(); + _chunkSubscription = null; + } +} + +class _MultiImageStreamCompleterHandle implements ImageStreamCompleterHandle { + _MultiImageStreamCompleterHandle(this._completer, this._delegateHandle) { + _completer!.__keepAliveHandles += 1; + } + + MultiImageStreamCompleter? _completer; + final ImageStreamCompleterHandle _delegateHandle; + + /// Call this method to signal the [ImageStreamCompleter] that it can now be + /// disposed when its last listener drops. + /// + /// This method must only be called once per object. + @override + void dispose() { + assert(_completer != null); + assert(_completer!.__keepAliveHandles > 0); + assert(!_completer!.__disposed); + + _delegateHandle.dispose(); + + _completer!.__keepAliveHandles -= 1; + _completer!.__maybeDispose(); + _completer = null; + } } diff --git a/cached_network_image/test/image_stream_completer_test.dart b/cached_network_image/test/image_stream_completer_test.dart index 2096c0fc..68ccda05 100644 --- a/cached_network_image/test/image_stream_completer_test.dart +++ b/cached_network_image/test/image_stream_completer_test.dart @@ -96,6 +96,37 @@ void main() { expect(tester.takeException(), 'failure message'); }); + test('Completer unsubscribes to chunk events when disposed', () async { + final codecStream = StreamController(); + final chunkStream = StreamController(); + + final MultiImageStreamCompleter completer = MultiImageStreamCompleter( + codec: codecStream.stream, + scale: 1.0, + chunkEvents: chunkStream.stream, + ); + + expect(chunkStream.hasListener, true); + + chunkStream.add( + const ImageChunkEvent(cumulativeBytesLoaded: 1, expectedTotalBytes: 3)); + + final ImageStreamListener listener = + ImageStreamListener((ImageInfo info, bool syncCall) {}); + // Cause the completer to dispose. + completer.addListener(listener); + completer.removeListener(listener); + + expect(chunkStream.hasListener, false); + + // The above expectation should cover this, but the point of this test is to + // make sure the completer does not assert that it's disposed and still + // receiving chunk events. Streams from the network can keep sending data + // even after evicting an image from the cache, for example. + chunkStream.add( + const ImageChunkEvent(cumulativeBytesLoaded: 2, expectedTotalBytes: 3)); + }); + testWidgets('Decoding starts when a listener is added after codec is ready', (WidgetTester tester) async { final codecStream = StreamController(); From ac60e35f39957782fa886840ee1504c5e4e8f1cd Mon Sep 17 00:00:00 2001 From: David Martos Date: Sat, 19 Nov 2022 16:54:38 +0100 Subject: [PATCH 3/3] fix typo --- .../lib/src/image_provider/multi_image_stream_completer.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cached_network_image/lib/src/image_provider/multi_image_stream_completer.dart b/cached_network_image/lib/src/image_provider/multi_image_stream_completer.dart index 1ceef802..53c2fa5e 100644 --- a/cached_network_image/lib/src/image_provider/multi_image_stream_completer.dart +++ b/cached_network_image/lib/src/image_provider/multi_image_stream_completer.dart @@ -70,7 +70,7 @@ class MultiImageStreamCompleter extends ImageStreamCompleter { // Used to guard against registering multiple _handleAppFrame callbacks for the same frame. bool _frameCallbackScheduled = false; - /// We must avoid disposing a completer if it has never had a listener, even + /// We must avoid disposing a completer if it never had a listener, even /// if all [keepAlive] handles get disposed. bool __hadAtLeastOneListener = false;