From 89714ce0759dabe11b53495c77ef164c6de1c677 Mon Sep 17 00:00:00 2001 From: Andrey Epin Date: Tue, 16 May 2023 15:22:32 -0700 Subject: Limit the number of parallel preview loadings in the image preview loader. With ag/23192997 the preview cache size effectively increased the total number of parallel image requests to a large enough value to DDoS Files app's content provider i.e. when sharing a large number of images, cache pre-population requests overlap with the actual preview loadings and causing some of them to fail to load. Update ImagePreviewImageLoader to use a Semaphore to limit the total number of parallel preview loadings. Fix: 283000541 Test: manual testing Change-Id: I6152f6e589a8b36a4810d617633017b72202e66f --- .../contentpreview/ImagePreviewImageLoader.kt | 29 +++++++++++++++++----- 1 file changed, 23 insertions(+), 6 deletions(-) (limited to 'java/src') diff --git a/java/src/com/android/intentresolver/contentpreview/ImagePreviewImageLoader.kt b/java/src/com/android/intentresolver/contentpreview/ImagePreviewImageLoader.kt index 89b79a0a..22dd1125 100644 --- a/java/src/com/android/intentresolver/contentpreview/ImagePreviewImageLoader.kt +++ b/java/src/com/android/intentresolver/contentpreview/ImagePreviewImageLoader.kt @@ -26,28 +26,42 @@ import androidx.annotation.VisibleForTesting import androidx.collection.LruCache import androidx.lifecycle.Lifecycle import androidx.lifecycle.coroutineScope +import java.util.function.Consumer import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Deferred import kotlinx.coroutines.isActive import kotlinx.coroutines.launch -import java.util.function.Consumer +import kotlinx.coroutines.sync.Semaphore private const val TAG = "ImagePreviewImageLoader" /** - * Implements preview image loading for the content preview UI. Provides requests deduplication and - * image caching. + * Implements preview image loading for the content preview UI. Provides requests deduplication, + * image caching, and a limit on the number of parallel loadings. */ @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE) -class ImagePreviewImageLoader( +class ImagePreviewImageLoader +@VisibleForTesting +constructor( private val scope: CoroutineScope, thumbnailSize: Int, private val contentResolver: ContentResolver, cacheSize: Int, + // TODO: consider providing a scope with the dispatcher configured with + // [CoroutineDispatcher#limitedParallelism] instead + private val contentResolverSemaphore: Semaphore, ) : ImageLoader { + constructor( + scope: CoroutineScope, + thumbnailSize: Int, + contentResolver: ContentResolver, + cacheSize: Int, + maxSimultaneousRequests: Int = 4 + ) : this(scope, thumbnailSize, contentResolver, cacheSize, Semaphore(maxSimultaneousRequests)) + private val thumbnailSize: Size = Size(thumbnailSize, thumbnailSize) private val lock = Any() @@ -103,13 +117,16 @@ class ImagePreviewImageLoader( } } - private fun RequestRecord.loadBitmap() { + private suspend fun RequestRecord.loadBitmap() { + contentResolverSemaphore.acquire() val bitmap = try { contentResolver.loadThumbnail(uri, thumbnailSize, null) } catch (t: Throwable) { Log.d(TAG, "failed to load $uri preview", t) null + } finally { + contentResolverSemaphore.release() } complete(bitmap) } @@ -136,4 +153,4 @@ class ImagePreviewImageLoader( val deferred: CompletableDeferred, @GuardedBy("lock") var caching: Boolean ) -} \ No newline at end of file +} -- cgit v1.2.3-59-g8ed1b