From 6c9f6531cfa55b6bd7c844ef2403fa0c694343c1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 5 Apr 2026 15:07:17 +0000 Subject: [PATCH] fix: prevent race conditions in thumbnail disk cache - Atomic writes: write to temp file then rename, so readers never see a partially written JPEG - Deduplication: ConcurrentHashMap tracks in-flight URLs so multiple composables requesting the same profile picture don't trigger duplicate decode+save work https://claude.ai/code/session_01PBJS3HDrurLP3i5n4tMsCv --- .../service/images/ProfilePictureFetcher.kt | 6 +-- .../service/images/ThumbnailDiskCache.kt | 46 +++++++++++++++---- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ProfilePictureFetcher.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ProfilePictureFetcher.kt index 9d0aa638b..0d8054399 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ProfilePictureFetcher.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ProfilePictureFetcher.kt @@ -99,11 +99,7 @@ class ProfilePictureFetcher( if (bitmap != null) { // Fire-and-forget: save thumbnail to disk for next load backgroundScope.launch { - try { - thumbnailCache.save(originalUrl, bitmap) - } catch (_: Exception) { - // Best-effort - } + thumbnailCache.save(originalUrl, bitmap) } return ImageFetchResult( diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ThumbnailDiskCache.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ThumbnailDiskCache.kt index 8cb2a79e0..e1f7f8f30 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ThumbnailDiskCache.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ThumbnailDiskCache.kt @@ -25,6 +25,7 @@ import android.graphics.BitmapFactory import com.vitorpamplona.quartz.nip01Core.core.toHexKey import com.vitorpamplona.quartz.utils.sha256.sha256 import java.io.File +import java.util.concurrent.ConcurrentHashMap /** * Disk cache for pre-resized profile picture thumbnails. @@ -33,8 +34,10 @@ import java.io.File * This avoids re-decoding large original images from Coil's disk cache on * every recomposition when images are evicted from the memory cache. * - * With 60 profile pictures on screen, reading ~300-600KB of tiny thumbnails - * is dramatically faster than re-decoding 60 full-size originals. + * Thread safety: + * - Writes use atomic temp-file-then-rename to prevent partial reads. + * - [inFlight] tracks URLs currently being saved to prevent duplicate work + * when multiple composables request the same profile picture simultaneously. */ class ThumbnailDiskCache( private val cacheDir: File, @@ -45,6 +48,9 @@ class ThumbnailDiskCache( const val JPEG_QUALITY = 80 } + // Tracks URLs currently being written to prevent duplicate saves + private val inFlight = ConcurrentHashMap.newKeySet() + init { cacheDir.mkdirs() } @@ -56,16 +62,40 @@ class ThumbnailDiskCache( return if (file.exists()) file else null } + /** + * Saves a bitmap as a JPEG thumbnail. Uses atomic write (temp file + rename) + * to prevent readers from seeing a partially written file. + * + * Returns true if saved, false if already in flight or on error. + */ fun save( url: String, bitmap: Bitmap, - ): File { - val file = File(cacheDir, keyFor(url)) - file.outputStream().use { out -> - bitmap.compress(Bitmap.CompressFormat.JPEG, JPEG_QUALITY, out) + ): Boolean { + // Skip if another coroutine is already saving this URL + if (!inFlight.add(url)) return false + + try { + val key = keyFor(url) + val finalFile = File(cacheDir, key) + + // Already saved by another coroutine that finished before us + if (finalFile.exists()) return true + + // Write to temp file, then atomically rename + val tempFile = File(cacheDir, "$key.tmp") + tempFile.outputStream().use { out -> + bitmap.compress(Bitmap.CompressFormat.JPEG, JPEG_QUALITY, out) + } + tempFile.renameTo(finalFile) + + evictIfNeeded() + return true + } catch (e: Exception) { + return false + } finally { + inFlight.remove(url) } - evictIfNeeded() - return file } fun decodeThumbnail(file: File): Bitmap? =