diff --git a/lib/services/image_cache_service.dart b/lib/services/image_cache_service.dart index 7ea1c246..1ce0b94a 100644 --- a/lib/services/image_cache_service.dart +++ b/lib/services/image_cache_service.dart @@ -6,6 +6,7 @@ import 'package:cached_network_image_ce/cached_network_image.dart' show FileResp // behind a narrower unsupported-platform stub. // ignore: implementation_imports import 'package:cached_network_image_ce/src/cache/default_cache_manager.dart' as ce_cache; +import 'package:flutter/foundation.dart' show visibleForTesting; import 'package:http/http.dart' as http; import 'package:path_provider/path_provider.dart'; @@ -33,7 +34,7 @@ class PlexImageCacheManager extends ce_cache.DefaultCacheManager { : super( stalePeriod: const Duration(days: 14), maxNrOfCacheObjects: 3000, - httpClientFactory: () => _SharedHttpClient(_artworkHttpClient.inner), + httpClientFactory: () => _SharedHttpClient(_artworkHttpClient.inner, _artworkRequestLimiter), cacheDirectoryProvider: getApplicationCacheDirectory, ); @@ -57,12 +58,13 @@ class PlexImageCacheManager extends ce_cache.DefaultCacheManager { /// transferring ownership of its lifecycle, and cap artwork fan-out globally. class _SharedHttpClient extends http.BaseClient { final http.Client _inner; + final _RequestLimiter _limiter; - _SharedHttpClient(this._inner); + _SharedHttpClient(this._inner, this._limiter); @override Future send(http.BaseRequest request) async { - final permit = await _artworkRequestLimiter.acquire(); + final permit = await _limiter.acquire(); var released = false; void release() { @@ -73,6 +75,29 @@ class _SharedHttpClient extends http.BaseClient { try { final response = await _inner.send(request); + + // CE's cache manager throws for any status other than 200/202 without + // listening to the body, so _releaseWhenDone would never fire and the + // permit would leak; six stale-thumb 404s then wedge all artwork loading + // until restart (#1473). Release now, drain the (tiny) error body in the + // background so the platform client reclaims the connection, and hand CE + // an empty body it never reads anyway. Status set mirrors CE 4.6.4 + // _downloadFile; recheck if the pinned dep is ever bumped. + if (response.statusCode != 200 && response.statusCode != 202) { + release(); + unawaited(response.stream.drain().catchError((_) {})); + return http.StreamedResponse( + const Stream>.empty(), + response.statusCode, + contentLength: 0, + request: response.request, + headers: response.headers, + isRedirect: response.isRedirect, + persistentConnection: response.persistentConnection, + reasonPhrase: response.reasonPhrase, + ); + } + return http.StreamedResponse( _releaseWhenDone(response.stream, release), response.statusCode, @@ -93,6 +118,11 @@ class _SharedHttpClient extends http.BaseClient { void close() {} } +/// Test hook: builds the throttled artwork client with an isolated limiter. +@visibleForTesting +http.Client createArtworkHttpClientForTest(http.Client inner, {int maxConcurrent = 6}) => + _SharedHttpClient(inner, _RequestLimiter(maxConcurrent)); + Stream> _releaseWhenDone(Stream> stream, void Function() release) async* { try { await for (final chunk in stream) { diff --git a/test/services/image_cache_service_test.dart b/test/services/image_cache_service_test.dart new file mode 100644 index 00000000..d2525158 --- /dev/null +++ b/test/services/image_cache_service_test.dart @@ -0,0 +1,104 @@ +import 'dart:async'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:plezy/services/image_cache_service.dart'; + +/// Pins the artwork request limiter's permit lifecycle. CE's cache manager +/// abandons non-200/202 responses without listening to the body, so the +/// wrapper must release those slots itself or the limiter wedges after a +/// handful of stale-thumb 404s (#1473). +void main() { + const timeout = Duration(seconds: 5); + + http.Request get(String path) => http.Request('GET', Uri.parse('https://example.invalid$path')); + + MockClient mixedClient() => MockClient((request) async { + if (request.url.path.contains('missing')) { + return http.Response('not found', 404, headers: {'x-marker': 'err'}); + } + return http.Response('poster-bytes', 200); + }); + + test('error burst does not wedge the limiter (#1473)', () async { + final client = createArtworkHttpClientForTest(mixedClient(), maxConcurrent: 2); + + // More failures than permits, bodies deliberately never read — mirrors + // CE throwing on the status without listening to the stream. + for (var i = 0; i < 5; i++) { + final response = await client.send(get('/missing/$i')).timeout(timeout); + expect(response.statusCode, 404); + expect(response.headers['x-marker'], 'err'); + } + + final ok = await client.send(get('/poster')).timeout(timeout); + expect(ok.statusCode, 200); + expect(await ok.stream.bytesToString().timeout(timeout), 'poster-bytes'); + }); + + test('successful downloads still hold a slot until the body is drained', () async { + final client = createArtworkHttpClientForTest(mixedClient(), maxConcurrent: 1); + + final first = await client.send(get('/poster')).timeout(timeout); + var secondDone = false; + final second = client.send(get('/poster')).then((response) async { + secondDone = true; + await response.stream.drain(); + }); + + await pumpEventQueue(); + expect(secondDone, isFalse, reason: 'slot must stay held while the first body is unread'); + + await first.stream.drain(); + await second.timeout(timeout); + expect(secondDone, isTrue); + }); + + test('error bodies are drained so the transport can reclaim the connection', () async { + final listened = Completer(); + final body = StreamController>(onListen: () => listened.complete()); + unawaited(body.close()); + final client = createArtworkHttpClientForTest( + MockClient.streaming((request, _) async => http.StreamedResponse(body.stream, 404)), + maxConcurrent: 2, + ); + + final response = await client.send(get('/missing')).timeout(timeout); + expect(response.statusCode, 404); + await listened.future.timeout(timeout); + }); + + test('a throwing send releases its slot', () async { + var failures = 0; + final client = createArtworkHttpClientForTest( + MockClient((request) async { + if (failures < 2) { + failures++; + throw http.ClientException('boom'); + } + return http.Response('poster-bytes', 200); + }), + maxConcurrent: 1, + ); + + for (var i = 0; i < 2; i++) { + await expectLater(client.send(get('/poster')).timeout(timeout), throwsA(isA())); + } + + final ok = await client.send(get('/poster')).timeout(timeout); + expect(ok.statusCode, 200); + }); + + test('cancelling a successful body releases its slot', () async { + final client = createArtworkHttpClientForTest(mixedClient(), maxConcurrent: 1); + + final first = await client.send(get('/poster')).timeout(timeout); + final subscription = first.stream.listen((_) {}); + await subscription.cancel(); + + final ok = await client.send(get('/poster')).timeout(timeout); + expect(ok.statusCode, 200); + await ok.stream.drain(); + }); +}