Merge pull request #4062 from vitorpamplona/claude/gif-loading-freeze-yt06sl

fix(images): stop copying whole GIFs into RAM before decoding them
This commit is contained in:
Vitor Pamplona
2026-09-07 21:56:39 -04:00
committed by GitHub
13 changed files with 1422 additions and 7 deletions
@@ -0,0 +1,137 @@
/*
* Copyright (c) 2025 Vitor Pamplona
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of
* this software and associated documentation files (the "Software"), to deal in
* the Software without restriction, including without limitation the rights to use,
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
* Software, and to permit persons to whom the Software is furnished to do so,
* subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/
package com.vitorpamplona.amethyst.service.images
import android.graphics.Bitmap
import android.os.Build
import androidx.core.graphics.createBitmap
import androidx.test.ext.junit.runners.AndroidJUnit4
import coil3.ImageLoader
import coil3.annotation.InternalCoilApi
import coil3.decode.DataSource
import coil3.decode.ImageSource
import coil3.decode.StaticImageDecoder
import coil3.decode.toImageDecoderSourceOrNull
import coil3.fetch.SourceFetchResult
import coil3.request.Options
import com.vitorpamplona.amethyst.AvifInstrumentedTestSupport.appContext
import com.vitorpamplona.amethyst.commons.service.image.DeferredDeleteFileSystem
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel
import okio.FileSystem
import okio.Path
import okio.Path.Companion.toOkioPath
import org.junit.After
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assume.assumeTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import java.io.File
import java.util.UUID
/**
* Pins the platform-side half of [SystemFileSystemFetcher], which the JVM unit tests cannot
* reach: `android.graphics.ImageDecoder` is what Coil's identity check gates access to, so only
* a device can show that the check really does fail on our disk cache's file system and really
* does pass once the source is re-homed.
*
* If these ever start passing without the re-home, Coil has loosened the check and
* [SystemFileSystemFetcher] can go.
*/
@RunWith(AndroidJUnit4::class)
class SystemFileSystemImageDecoderInstrumentedTest {
private lateinit var pngFile: File
private lateinit var path: Path
private lateinit var scope: CoroutineScope
private lateinit var deferredDelete: DeferredDeleteFileSystem
private val imageLoader by lazy { ImageLoader.Builder(appContext).build() }
private val options by lazy { Options(appContext) }
@Before
fun setUp() {
pngFile = File(appContext.cacheDir.also { it.mkdirs() }, "${UUID.randomUUID()}.png")
pngFile.outputStream().use { createBitmap(4, 4).compress(Bitmap.CompressFormat.PNG, 100, it) }
path = pngFile.toOkioPath()
scope = CoroutineScope(Dispatchers.IO + SupervisorJob())
deferredDelete = DeferredDeleteFileSystem(FileSystem.SYSTEM, scope)
}
@After
fun tearDown() {
scope.cancel()
pngFile.delete()
}
/** How Coil's `NetworkFetcher` builds the source when the disk cache wraps its file system. */
private fun diskCacheSource() = ImageSource(file = path, fileSystem = deferredDelete, diskCacheKey = KEY)
private fun reHomedSource() = requireNotNull(diskCacheSource().onSystemFileSystem(KEY))
@OptIn(InternalCoilApi::class)
@Test
fun ourDiskCacheFileSystemHidesTheFileFromImageDecoder() {
assertNull(
"still decode path",
diskCacheSource().toImageDecoderSourceOrNull(options, animated = false),
)
assertNull(
"animated decode path",
diskCacheSource().toImageDecoderSourceOrNull(options, animated = true),
)
}
@OptIn(InternalCoilApi::class)
@Test
fun theReHomedSourceReachesImageDecoder() {
assertNotNull(
"still decode path",
reHomedSource().toImageDecoderSourceOrNull(options, animated = false),
)
assertNotNull(
"animated decode path",
reHomedSource().toImageDecoderSourceOrNull(options, animated = true),
)
}
@Test
fun staticImageDecoderDeclinesOurDiskCacheSourceAndAcceptsTheReHomedOne() {
assumeTrue("StaticImageDecoder requires API 29+", Build.VERSION.SDK_INT >= 29)
// Declining is why every still image in the app was decoding through BitmapFactoryDecoder.
assertNull(
StaticImageDecoder.Factory().create(fetchResult(diskCacheSource()), options, imageLoader),
)
assertNotNull(
StaticImageDecoder.Factory().create(fetchResult(reHomedSource()), options, imageLoader),
)
}
private fun fetchResult(source: ImageSource) = SourceFetchResult(source, "image/png", DataSource.DISK)
companion object {
private const val KEY = "https://example.com/blob.png"
}
}
@@ -82,6 +82,7 @@ import com.vitorpamplona.amethyst.service.crashreports.CrashReportCache
import com.vitorpamplona.amethyst.service.crashreports.UnexpectedCrashSaver
import com.vitorpamplona.amethyst.service.eventCache.MemoryTrimmingService
import com.vitorpamplona.amethyst.service.images.ImageCacheFactory
import com.vitorpamplona.amethyst.service.images.ImageDiskCacheReconciler
import com.vitorpamplona.amethyst.service.images.ImageLoaderSetup
import com.vitorpamplona.amethyst.service.images.ThumbnailDiskCache
import com.vitorpamplona.amethyst.service.location.LocationState
@@ -1136,6 +1137,22 @@ class AppModules(
}
}
// Reclaim image-cache files orphaned by a process death that lost DeferredDeleteFileSystem's
// queued unlinks — Coil cannot see them, so without this the directory keeps every killed
// process's residue and drifts past its own cap forever. See ImageDiskCacheReconciler.
//
// Rate-limited to once a day: drift accrues over process deaths, not over startups, and this
// runs on every one of them — including the WorkManager wake-ups that cold-start the graph.
//
// Also the one place that forces the `diskCache` lazy, so its build (a statvfs for the size
// budget) and the check both land on IO rather than on whichever thread loads an image first.
applicationIOScope.launch {
val result = ImageDiskCacheReconciler.reconcileIfDue(diskCache)
if (result != null && result.wasOverBudget) {
Log.i("AppModules") { "Image cache was over budget: wiped ${result.reclaimedFiles} files (${result.bytesOnDisk} bytes on disk, ${result.budgetBytes} budget)" }
}
}
applicationIOScope.launch {
// loads main account quickly.
LocalPreferences.loadAccountConfigFromEncryptedStorage()
@@ -0,0 +1,100 @@
/*
* Copyright (c) 2025 Vitor Pamplona
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of
* this software and associated documentation files (the "Software"), to deal in
* the Software without restriction, including without limitation the rights to use,
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
* Software, and to permit persons to whom the Software is furnished to do so,
* subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/
package com.vitorpamplona.amethyst.service.images
import android.os.Build.VERSION.SDK_INT
import androidx.annotation.RequiresApi
import coil3.ImageLoader
import coil3.decode.DecodeUtils
import coil3.decode.Decoder
import coil3.decode.ImageSource
import coil3.fetch.SourceFetchResult
import coil3.gif.AnimatedImageDecoder
import coil3.gif.isAnimatedHeif
import coil3.gif.isAnimatedWebP
import coil3.gif.isGif
import coil3.request.Options
import okio.BufferedSource
import okio.buffer
/**
* Drop-in replacement for Coil's `AnimatedImageDecoder.Factory` that stops asking for the GIF
* frame-delay rewrite when the file does not need it.
*
* The sniffing is identical to Coil's. The one difference: below API 34 Coil wraps every GIF in a
* stream-backed `FrameDelayRewritingSource` to clamp sub-threshold frame delays, and a
* stream-backed source has no file for `ImageDecoder` to read, so the decode falls back to
* squashing the whole encoded animation into RAM (see [SystemFileSystemFetcher] for what that
* costs). Nearly every GIF comes out of that rewriter byte-for-byte identical, so
* [hasSubThresholdGifFrameDelay] checks first and the rewrite is requested only for the files
* that would actually change.
*/
@RequiresApi(28)
class AnimatedImageDecoderFactory : Decoder.Factory {
override fun create(
result: SourceFetchResult,
options: Options,
imageLoader: ImageLoader,
): Decoder? {
val source = result.source.source()
val isGif = DecodeUtils.isGif(source)
if (!isGif && !isAnimatedNonGif(source)) return null
return newAnimatedImageDecoder(result.source, options, isGif)
}
private fun isAnimatedNonGif(source: BufferedSource): Boolean = DecodeUtils.isAnimatedWebP(source) || (SDK_INT >= 30 && DecodeUtils.isAnimatedHeif(source))
}
/**
* Builds Coil's [AnimatedImageDecoder], asking for the frame-delay rewrite only when it is both
* possible and needed.
*
* [mayNeedFrameDelayRewrite] should be true only for GIFs: Coil's rewriter no-ops on every other
* format, so scanning one would be pure IO for a decision already made.
*/
@RequiresApi(28)
fun newAnimatedImageDecoder(
source: ImageSource,
options: Options,
mayNeedFrameDelayRewrite: Boolean,
): Decoder = AnimatedImageDecoder(source, options, enforceMinimumFrameDelay(source, mayNeedFrameDelayRewrite))
/**
* Whether to ask Coil to clamp this GIF's sub-threshold frame delays. From API 34 the platform
* decoder does it itself, which is why Coil's own default turns the rewrite off there; below that
* we pay for it only when the file really has a delay to clamp.
*/
private fun enforceMinimumFrameDelay(
source: ImageSource,
mayNeedFrameDelayRewrite: Boolean,
): Boolean {
if (!mayNeedFrameDelayRewrite || SDK_INT >= 34) return false
// Not file-backed: the decode squashes the stream into RAM either way, so there is no fast
// path to protect and no reason to deviate from Coil's default.
val file = source.fileOrNull() ?: return true
return source.fileSystem
.source(file)
.buffer()
.use { it.hasSubThresholdGifFrameDelay() }
}
@@ -41,7 +41,8 @@ import okio.ByteString.Companion.encodeUtf8
*
* On API 31+, the platform [android.graphics.ImageDecoder] produces an
* [android.graphics.drawable.AnimatedImageDrawable] for animated AVIF. We delegate the
* actual decode to Coil's [AnimatedImageDecoder] — only the brand sniff is custom.
* actual decode to Coil's [AnimatedImageDecoder] — only the brand sniff is custom. It is built
* through [newAnimatedImageDecoder] so an AVIF gets the same disk-cache file fast path a GIF does.
*
* Below API 31 the platform decoder cannot handle AVIF at all, so we decline and let
* other decoders try (they will also fail, and Coil falls through to its error slot,
@@ -62,7 +63,7 @@ class AvifAnimatedDecoderFactory : Decoder.Factory {
private fun createAnimatedImageDecoder(
result: SourceFetchResult,
options: Options,
): Decoder = AnimatedImageDecoder(result.source, options)
): Decoder = newAnimatedImageDecoder(result.source, options, mayNeedFrameDelayRewrite = false)
private fun isAvif(source: BufferedSource): Boolean =
source.rangeEquals(4, FTYP) &&
@@ -89,7 +89,9 @@ class BlossomFetcher(
connectivityChecker = lazy { connectivityCheckerLazy.get(options.context) },
concurrentRequestStrategy = concurrentRequestStrategyLazy,
)
}
// Keyed on the resolved server url, which is what the NetworkFetcher above
// caches under -- not on the `blossom:` uri the request came in as.
}.onSystemFileSystem(options.diskCacheKey ?: url)
}
}
@@ -0,0 +1,92 @@
/*
* Copyright (c) 2025 Vitor Pamplona
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of
* this software and associated documentation files (the "Software"), to deal in
* the Software without restriction, including without limitation the rights to use,
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
* Software, and to permit persons to whom the Software is furnished to do so,
* subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/
package com.vitorpamplona.amethyst.service.images
import okio.BufferedSource
/**
* Frame delay, in hundredths of a second, below which Coil rewrites a GIF's graphics control
* blocks. Mirrors `FrameDelayRewritingSource.MINIMUM_FRAME_DELAY` in Coil 3.5.0.
*/
private const val MINIMUM_FRAME_DELAY = 2
/** `00 21 F9 04` — block terminator, extension introducer, graphics control label, block size. */
private const val MARKER_0: Byte = 0x00
private const val MARKER_1: Byte = 0x21
private const val MARKER_2: Byte = 0xF9.toByte()
private const val MARKER_3: Byte = 0x04
/**
* Bytes one graphics control block occupies from the first marker byte: the 4 marker bytes,
* the packed field, the two delay bytes, the transparent colour index and the terminator.
*/
private const val BLOCK_SIZE = 9
private const val SCAN_CHUNK = 64 * 1024
/**
* Streams over a GIF looking for a graphics control block whose frame delay is below
* [MINIMUM_FRAME_DELAY] — i.e. one that Coil's `FrameDelayRewritingSource` would actually
* rewrite.
*
* Coil wraps every GIF in that rewriting source on API < 34 (the platform clamps sub-threshold
* delays itself from 34 on). The wrapper is stream-backed, which costs the file fast path in
* [onSystemFileSystem] and forces the whole encoded animation into RAM twice. The overwhelming
* majority of GIFs declare a sane delay and come out of the rewriter byte-for-byte identical,
* so this scan buys the fast path back for them and leaves the rewrite in place only for the
* files that need it.
*
* The condition matches Coil's exactly (verified against Coil 3.5.0): a delay below the
* threshold in a block whose terminator byte is zero. Reading the whole stream costs one
* sequential pass over a [SCAN_CHUNK] buffer and allocates nothing else, against the
* file-sized heap **and** direct-buffer copies the rewrite path would otherwise make.
*/
fun BufferedSource.hasSubThresholdGifFrameDelay(): Boolean {
// Carries the last BLOCK_SIZE - 1 bytes of a chunk forward so a block that straddles a
// chunk boundary is still seen whole.
val window = ByteArray(SCAN_CHUNK + BLOCK_SIZE)
var carried = 0
while (true) {
val read = read(window, carried, SCAN_CHUNK)
if (read == -1) return false
val filled = carried + read
var i = 0
val last = filled - BLOCK_SIZE
while (i <= last) {
if (window[i] == MARKER_0 &&
window[i + 1] == MARKER_1 &&
window[i + 2] == MARKER_2 &&
window[i + 3] == MARKER_3 &&
window[i + 8] == 0.toByte()
) {
// Delay is two unsigned bytes, least significant first.
val delay = ((window[i + 6].toInt() and 0xFF) shl 8) or (window[i + 5].toInt() and 0xFF)
if (delay < MINIMUM_FRAME_DELAY) return true
}
i++
}
carried = minOf(filled, BLOCK_SIZE - 1)
window.copyInto(window, 0, filled - carried, filled)
}
}
@@ -0,0 +1,112 @@
/*
* Copyright (c) 2025 Vitor Pamplona
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of
* this software and associated documentation files (the "Software"), to deal in
* the Software without restriction, including without limitation the rights to use,
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
* Software, and to permit persons to whom the Software is furnished to do so,
* subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/
package com.vitorpamplona.amethyst.service.images
import coil3.decode.ImageSource
import coil3.fetch.FetchResult
import coil3.fetch.Fetcher
import coil3.fetch.SourceFetchResult
import com.vitorpamplona.amethyst.commons.service.image.DeferredDeleteFileSystem
import okio.FileSystem
/**
* Wraps a network [Fetcher] so the disk-cache file it hands back is addressed through
* [FileSystem.SYSTEM] itself instead of through our [DeferredDeleteFileSystem] wrapper.
*
* Coil reaches for the platform `android.graphics.ImageDecoder` only when an image source's
* file system is **referentially** [FileSystem.SYSTEM] (`ImageSource.toImageDecoderSourceOrNull`,
* verified against Coil 3.5.0):
*
* ```
* if (fileSystem === FileSystem.SYSTEM) {
* val file = fileOrNull()
* if (file != null) return ImageDecoder.createSource(file.toFile())
* }
* ```
*
* Coil's `NetworkFetcher` stamps the source it builds with `diskCache.fileSystem`, and ours is a
* [DeferredDeleteFileSystem] wrapper, so that identity check failed for **every** image the app
* fetched from the network. Two decoders quietly lost their fast path because of it:
*
* - `AnimatedImageDecoder` fell back to
* `ImageDecoder.createSource(source.squashToDirectByteBuffer())`, pulling the whole encoded
* animation onto the heap and then copying it into an equally large direct `ByteBuffer` that
* lives as long as the `AnimatedImageDrawable`. Replaying that over a 69.8 MB GIF measured
* 66 MB of heap plus 66 MB of native memory, and animated results are never memory-cached
* (`DrawableImage.shareable` is false), so the feed paid it again on every scroll back.
* - `StaticImageDecoder.Factory` declined outright (it returns null when it cannot get an
* `ImageDecoder.Source`), so every still image decoded through `BitmapFactoryDecoder`.
*
* Only deletes are deferred by the wrapper — reads already go straight to [FileSystem.SYSTEM] —
* so handing decoders the same file on the real system file system is equivalent, and it is what
* Coil does when no wrapper is installed.
*
* This sits at the fetcher rather than at each decoder on purpose: one seam fixes every decoder
* at once, it keeps Coil's own decoder registration (and with it the parallelism semaphore the
* bitmap decoders share), and it is the only place that still knows [diskCacheKey], which
* `FileImageSource` exposes internally and cannot be copied off an existing source.
*/
class SystemFileSystemFetcher(
private val delegate: Fetcher,
private val diskCacheKey: String,
) : Fetcher {
override suspend fun fetch(): FetchResult? {
val result = delegate.fetch()
if (result !is SourceFetchResult) return result
val onSystem = result.source.onSystemFileSystem(diskCacheKey) ?: return result
return SourceFetchResult(onSystem, result.mimeType, result.dataSource)
}
}
/** This fetcher, with any disk-cache-backed result re-homed onto [FileSystem.SYSTEM]. */
fun Fetcher.onSystemFileSystem(diskCacheKey: String): Fetcher = SystemFileSystemFetcher(this, diskCacheKey)
/**
* The same bytes, addressed through [FileSystem.SYSTEM], or null when there is nothing to gain:
* already on it, wrapped in something other than a [DeferredDeleteFileSystem] (which may rewrite
* paths or serve different bytes, so the file underneath is not ours to hand out), or not backed
* by a file yet.
*
* [ImageSource.fileOrNull] is used rather than [ImageSource.file] on purpose — the latter would
* materialise a temp copy of a stream-backed source, which is the very cost this exists to avoid.
*/
fun ImageSource.onSystemFileSystem(diskCacheKey: String): ImageSource? {
if (fileSystem === FileSystem.SYSTEM) return null
var unwrapped: FileSystem = fileSystem
while (unwrapped is DeferredDeleteFileSystem) unwrapped = unwrapped.delegate
if (unwrapped !== FileSystem.SYSTEM) return null
val file = fileOrNull() ?: return null
return ImageSource(
file = file,
fileSystem = FileSystem.SYSTEM,
diskCacheKey = diskCacheKey,
// Takes ownership of the source it replaces. Only the returned source reaches the engine,
// and the engine closes exactly one source per fetch, so this is what releases the
// disk-cache snapshot holding `file` open.
closeable = this,
metadata = metadata,
)
}
@@ -0,0 +1,220 @@
/*
* Copyright (c) 2025 Vitor Pamplona
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of
* this software and associated documentation files (the "Software"), to deal in
* the Software without restriction, including without limitation the rights to use,
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
* Software, and to permit persons to whom the Software is furnished to do so,
* subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/
package com.vitorpamplona.amethyst.service.images
import coil3.disk.DiskCache
import com.vitorpamplona.amethyst.commons.service.image.DeferredDeleteFileSystem
import com.vitorpamplona.quartz.utils.Log
import okio.IOException
import okio.Path
/** What one [ImageDiskCacheReconciler] pass found, and how many files it unlinked. */
data class ImageCacheReconciliation(
val bytesOnDisk: Long,
val budgetBytes: Long,
val ceilingBytes: Long,
val reclaimedFiles: Int,
) {
val wasOverBudget get() = reclaimedFiles > 0
}
/**
* Brings the image cache directory back under its own size budget at startup.
*
* [DeferredDeleteFileSystem] moves Coil's eviction `unlink()` off the `DiskLruCache` lock by
* queueing it in memory. Anything still queued when the process dies is never unlinked — and Coil
* cannot recover it, because `DiskLruCache.processJournal()` computes `size` purely from the
* journal's recorded lengths and never scans the directory for files it does not know about. The
* orphan counts toward neither `size` nor eviction, so the directory keeps whatever residue every
* killed process left behind, forever.
*
* Measured against the real Coil `DiskCache`: 48 unlinks lost to one process death left Coil
* reporting 16 KB against a 16 KB budget while the directory actually held 48 KB — and reopening
* as a fresh process reclaimed none of it.
*
* `DiskCache.clear()` is not enough on its own: it evicts `lruEntries`, which is exactly the set of
* files the journal knows about, so it walks straight past the orphans. Hence the two steps below —
* empty the journal, then unlink whatever is still sitting in the directory.
*
* This is deliberately blunt. It costs the whole cache, so it only fires once the directory has
* drifted past [ceilingBytes] — well beyond the slack normal operation needs. It does not read the
* journal, so it stays independent of Coil's on-disk format.
*
* Drift accrues only when a process dies with unlinks still queued, which is slow, so
* [reconcileIfDue] rate-limits the check to [DEFAULT_INTERVAL_MS]. Even the healthy pass is a
* `readdir` plus a `stat` per file — on a full 1 GB cache, tens of thousands of syscalls — and
* `AppModules.initiate()` runs on every process start, including the WorkManager wake-ups that
* cold-start the whole graph. Gating it on a marker file's mtime costs one `stat` on the starts
* that skip.
*/
object ImageDiskCacheReconciler {
/**
* Fraction of the cache's own budget allowed on top of it before the directory counts as
* drifted rather than merely busy.
*/
private const val DEFAULT_SLACK_FRACTION = 0.25
/**
* Floor for that slack, so a cache with a small budget (20% of a nearly-full disk) is not wiped
* over a few megabytes. Covers the journal plus the dirty files of any writes in flight.
*/
private const val DEFAULT_MIN_SLACK_BYTES = 4L * 1024 * 1024
/** How long one pass is good for. Drift accrues over days, so checking daily is ample. */
private const val DEFAULT_INTERVAL_MS = 24L * 60 * 60 * 1000
/**
* Empty file whose mtime is the last pass. It lives inside the cache directory so it travels
* with the thing it describes: clearing the app's cache from Settings takes the marker with it,
* and the next start reconciles a directory whose history we no longer know.
*/
private const val MARKER_FILE = ".reconciled"
/** Coil's own bookkeeping, which is not entry data and must survive a wipe. */
private val JOURNAL_FILES = setOf("journal", "journal.tmp", "journal.bkp")
/** Files in the cache directory that are not entry data and must survive a wipe. */
private val PRESERVED_FILES = JOURNAL_FILES + MARKER_FILE
/** Bytes the directory may hold before [reconcile] wipes it. */
fun ceilingBytes(
budgetBytes: Long,
slackFraction: Double = DEFAULT_SLACK_FRACTION,
minSlackBytes: Long = DEFAULT_MIN_SLACK_BYTES,
): Long = budgetBytes + maxOf((budgetBytes * slackFraction).toLong(), minSlackBytes)
/**
* Runs [reconcile] if the last pass is older than [intervalMs], else returns null having done
* one `stat`.
*
* Blocking IO — call it from a background dispatcher.
*/
fun reconcileIfDue(
diskCache: DiskCache,
now: Long = System.currentTimeMillis(),
intervalMs: Long = DEFAULT_INTERVAL_MS,
slackFraction: Double = DEFAULT_SLACK_FRACTION,
minSlackBytes: Long = DEFAULT_MIN_SLACK_BYTES,
): ImageCacheReconciliation? {
if (!isDue(diskCache, now, intervalMs)) return null
return reconcile(diskCache, slackFraction, minSlackBytes).also { markPass(diskCache) }
}
/**
* True when no pass is recorded, or the recorded one is [intervalMs] old.
*
* A marker dated in the future — a clock that jumped back, or a restored backup — would
* otherwise park the check until real time caught up, so that also counts as due.
*/
private fun isDue(
diskCache: DiskCache,
now: Long,
intervalMs: Long,
): Boolean {
val lastPass =
try {
diskCache.fileSystem.metadataOrNull(diskCache.directory / MARKER_FILE)?.lastModifiedAtMillis
} catch (e: IOException) {
Log.d("ImageDiskCache") { "could not stat the marker: ${e.message}" }
null
} ?: return true
return now - lastPass >= intervalMs || lastPass > now
}
/** Records that a pass just happened, by writing the marker's mtime to now. */
private fun markPass(diskCache: DiskCache) {
try {
diskCache.fileSystem.createDirectories(diskCache.directory)
diskCache.fileSystem.write(diskCache.directory / MARKER_FILE) {}
} catch (e: IOException) {
// Losing the marker only costs a redundant walk on the next start.
Log.d("ImageDiskCache") { "could not write the marker: ${e.message}" }
}
}
/**
* Walks the cache directory and, if it holds more than [ceilingBytes], empties it.
*
* Blocking IO — call it from a background dispatcher. Prefer [reconcileIfDue]; this is the
* unconditional pass.
*
* Deliberately does not read `DiskCache.size`: that would force the journal parse on the happy
* path, and the decision does not need it. A request racing the wipe can lose the entry it was
* writing; Coil treats that as a cache miss and re-fetches, which is why this runs at startup
* rather than while the feed is scrolling.
*/
fun reconcile(
diskCache: DiskCache,
slackFraction: Double = DEFAULT_SLACK_FRACTION,
minSlackBytes: Long = DEFAULT_MIN_SLACK_BYTES,
): ImageCacheReconciliation {
val budget = diskCache.maxSize
val ceiling = ceilingBytes(budget, slackFraction, minSlackBytes)
val bytesOnDisk = regularFiles(diskCache).sumOf { sizeOf(diskCache, it) }
if (bytesOnDisk <= ceiling) {
return ImageCacheReconciliation(bytesOnDisk, budget, ceiling, 0)
}
Log.w(
"ImageDiskCache",
"Image cache holds $bytesOnDisk bytes against a $budget budget (ceiling $ceiling) — " +
"wiping it. Deferred unlinks lost to a process death leave files Coil can no longer see.",
)
// Empties the journal, so every entry file left below is unreferenced by definition.
// Its own unlinks go through the deferred file system like any other eviction; the paths
// below are re-queued rather than double-unlinked, which the pending set dedupes.
diskCache.clear()
var reclaimed = 0
regularFiles(diskCache).forEach { path ->
if (path.name in PRESERVED_FILES) return@forEach
try {
diskCache.fileSystem.delete(path, mustExist = false)
reclaimed++
} catch (e: IOException) {
Log.w("ImageDiskCache", "could not unlink $path", e)
}
}
return ImageCacheReconciliation(bytesOnDisk, budget, ceiling, reclaimed)
}
private fun regularFiles(diskCache: DiskCache): List<Path> =
try {
diskCache.fileSystem
.listOrNull(diskCache.directory)
.orEmpty()
.filter { diskCache.fileSystem.metadataOrNull(it)?.isRegularFile == true }
} catch (e: IOException) {
// A missing directory is the normal first-launch state, not a failure.
Log.d("ImageDiskCache") { "could not list ${diskCache.directory}: ${e.message}" }
emptyList()
}
private fun sizeOf(
diskCache: DiskCache,
path: Path,
): Long = diskCache.fileSystem.metadataOrNull(path)?.size ?: 0L
}
@@ -29,7 +29,6 @@ import coil3.annotation.DelicateCoilApi
import coil3.annotation.ExperimentalCoilApi
import coil3.disk.DiskCache
import coil3.fetch.Fetcher
import coil3.gif.AnimatedImageDecoder
import coil3.gif.GifDecoder
import coil3.memory.MemoryCache
import coil3.network.CacheStrategy
@@ -61,7 +60,9 @@ class ImageLoaderSetup {
companion object {
val gifFactory =
if (Build.VERSION.SDK_INT >= 28) {
AnimatedImageDecoder.Factory()
// Not Coil's AnimatedImageDecoder.Factory: see [AnimatedImageDecoderFactory] for
// why decoding straight from the disk-cache file matters here.
AnimatedImageDecoderFactory()
} else {
GifDecoder.Factory()
}
@@ -169,6 +170,9 @@ class OkHttpFactory(
val url = data.toString()
// onSystemFileSystem keeps the platform ImageDecoder reachable for whatever comes back
// -- see [SystemFileSystemFetcher]. The key it carries is the one NetworkFetcher derives
// for the same request (`options.diskCacheKey ?: url`).
return readAuthAware(url, readAuth) { authHeader ->
NetworkFetcher(
url = url,
@@ -179,7 +183,7 @@ class OkHttpFactory(
connectivityChecker = lazy { connectivityCheckerLazy.get(options.context) },
concurrentRequestStrategy = concurrentRequestStrategyLazy,
)
}
}.onSystemFileSystem(options.diskCacheKey ?: url)
}
private fun isApplicable(data: Uri): Boolean = data.scheme == "http" || data.scheme == "https"
@@ -127,7 +127,7 @@ class ProfilePictureFetcher(
connectivityChecker = lazy { connectivityCheckerLazy.get(options.context) },
concurrentRequestStrategy = concurrentRequestStrategyLazy,
)
}
}.onSystemFileSystem(options.diskCacheKey ?: data.url)
return ProfilePictureFetcher(
data.url,
@@ -0,0 +1,132 @@
/*
* Copyright (c) 2025 Vitor Pamplona
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of
* this software and associated documentation files (the "Software"), to deal in
* the Software without restriction, including without limitation the rights to use,
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
* Software, and to permit persons to whom the Software is furnished to do so,
* subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/
package com.vitorpamplona.amethyst.service.images
import okio.Buffer
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Test
/**
* The scan decides whether a GIF still needs Coil's in-RAM frame-delay rewrite on API < 34, so
* a false negative would silently change how a GIF animates. Its condition therefore has to
* match `FrameDelayRewritingSource` byte for byte: a delay below 2/100 s in a graphics control
* block whose terminator is zero.
*/
class GifFrameDelaysTest {
/**
* `00 21 F9 04` marker, packed field, delay (low, high), transparent colour index, block
* terminator — the nine bytes Coil inspects.
*/
private fun graphicsControlBlock(
delay: Int,
terminator: Int = 0,
) = byteArrayOf(
0x00,
0x21,
0xF9.toByte(),
0x04,
0x00,
(delay and 0xFF).toByte(),
((delay shr 8) and 0xFF).toByte(),
0x00,
terminator.toByte(),
)
private fun gif(vararg blocks: ByteArray) =
Buffer().apply {
write("GIF89a".toByteArray())
write(ByteArray(7)) // logical screen descriptor
blocks.forEach { write(it) }
writeByte(0x3B) // trailer
}
@Test
fun aSaneDelayNeedsNoRewrite() {
// 5/100 s is what the 69.8 MB GIF from the bug report declares on all 201 frames.
assertFalse(gif(graphicsControlBlock(delay = 5)).hasSubThresholdGifFrameDelay())
}
@Test
fun zeroAndOneHundredthsAreBelowTheThreshold() {
assertTrue(gif(graphicsControlBlock(delay = 0)).hasSubThresholdGifFrameDelay())
assertTrue(gif(graphicsControlBlock(delay = 1)).hasSubThresholdGifFrameDelay())
}
@Test
fun twoHundredthsIsTheFirstAcceptedDelay() {
assertFalse(gif(graphicsControlBlock(delay = 2)).hasSubThresholdGifFrameDelay())
}
@Test
fun aDelayAboveOneByteIsReadLeastSignificantByteFirst() {
// 0x0100 = 256/100 s. Reading the bytes in the wrong order would see 1 and rewrite.
assertFalse(gif(graphicsControlBlock(delay = 256)).hasSubThresholdGifFrameDelay())
}
@Test
fun oneBadBlockAmongGoodOnesIsEnough() {
val stream =
gif(
graphicsControlBlock(delay = 10),
graphicsControlBlock(delay = 10),
graphicsControlBlock(delay = 0),
graphicsControlBlock(delay = 10),
)
assertTrue(stream.hasSubThresholdGifFrameDelay())
}
@Test
fun aNonZeroTerminatorIsSkipped() {
// Coil bails out of the rewrite when the block does not end where it expects, so a
// marker-shaped run of bytes inside image data must not count as a frame delay.
assertFalse(gif(graphicsControlBlock(delay = 0, terminator = 0x2C)).hasSubThresholdGifFrameDelay())
}
@Test
fun aBlockStraddlingTheScanChunkBoundaryIsStillFound() {
// The scan reads in 64 KiB chunks; without the carried-over window a block landing on
// the seam would be missed and the GIF would animate at the wrong speed.
for (offset in 0..8) {
val padding = ByteArray(64 * 1024 - 8 + offset)
val stream =
Buffer().apply {
write(padding)
write(graphicsControlBlock(delay = 0))
}
assertTrue("block at 64KiB - 8 + $offset", stream.hasSubThresholdGifFrameDelay())
}
}
@Test
fun anEmptySourceNeedsNoRewrite() {
assertFalse(Buffer().hasSubThresholdGifFrameDelay())
}
@Test
fun aTruncatedBlockAtTheEndOfTheStreamNeedsNoRewrite() {
val stream = Buffer().apply { write(graphicsControlBlock(delay = 0).copyOfRange(0, 8)) }
assertFalse(stream.hasSubThresholdGifFrameDelay())
}
}
@@ -0,0 +1,367 @@
/*
* Copyright (c) 2025 Vitor Pamplona
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of
* this software and associated documentation files (the "Software"), to deal in
* the Software without restriction, including without limitation the rights to use,
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
* Software, and to permit persons to whom the Software is furnished to do so,
* subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/
package com.vitorpamplona.amethyst.service.images
import coil3.disk.DiskCache
import com.vitorpamplona.amethyst.commons.service.image.DeferredDeleteFileSystem
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel
import okio.FileSystem
import okio.Path
import okio.Path.Companion.toOkioPath
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import java.io.File
import java.nio.file.Files
/**
* The leak this guards against is silent from inside Coil: a deferred unlink lost to a process
* death leaves a file that `DiskLruCache.processJournal()` never counts and never evicts, because
* it derives `size` from the journal alone. So these tests measure the directory, not the cache.
*/
class ImageDiskCacheReconcilerTest {
private lateinit var tmpRoot: File
private lateinit var cacheDir: Path
private lateinit var scope: CoroutineScope
@Before
fun setUp() {
tmpRoot = Files.createTempDirectory("image-cache-reconciler-test").toFile()
cacheDir = tmpRoot.resolve("image_cache").toOkioPath()
scope = CoroutineScope(Dispatchers.IO + SupervisorJob())
}
@After
fun tearDown() {
scope.cancel()
tmpRoot.deleteRecursively()
}
private fun bytesOnDisk(): Long =
FileSystem.SYSTEM
.listOrNull(cacheDir)
.orEmpty()
.sumOf { FileSystem.SYSTEM.metadataOrNull(it)?.size ?: 0L }
private fun newCache(
fileSystem: FileSystem,
maxSize: Long,
) = DiskCache
.Builder()
.directory(cacheDir)
.fileSystem(fileSystem)
.maxSizeBytes(maxSize)
.build()
private fun writeEntry(
diskCache: DiskCache,
key: String,
bytes: Int,
) {
val editor = diskCache.openEditor(key) ?: error("openEditor returned null for $key")
try {
diskCache.fileSystem.write(editor.data) { write(ByteArray(bytes) { 1 }) }
editor.commit()
} catch (e: Throwable) {
editor.abort()
throw e
}
}
@Test
fun orphansLostToAProcessDeath_areReclaimed() {
// "Process one": an inert drainer stands in for a process that died with eviction's
// unlinks still queued in memory.
val deadProcessFs = DeferredDeleteFileSystem(FileSystem.SYSTEM, scope)
scope.cancel("the drainer never runs — the process died first")
val budget = 16L * 1024
val first = newCache(deadProcessFs, budget)
repeat(40) { i -> writeEntry(first, "key$i", 1024) }
val deadline = System.currentTimeMillis() + 5_000
while (first.size > budget && System.currentTimeMillis() < deadline) Thread.sleep(20)
first.shutdown()
val leaked = bytesOnDisk()
assertTrue(
"the orphans must actually exceed the ceiling, or the test proves nothing (onDisk=$leaked)",
leaked > ImageDiskCacheReconciler.ceilingBytes(budget, minSlackBytes = 0),
)
// "Process two": a fresh start over the same directory.
val liveScope = CoroutineScope(Dispatchers.IO + SupervisorJob())
val liveFs = DeferredDeleteFileSystem(FileSystem.SYSTEM, liveScope)
val second = newCache(liveFs, budget)
try {
val result = ImageDiskCacheReconciler.reconcile(second, minSlackBytes = 0)
assertTrue("should have judged the directory over budget", result.wasOverBudget)
assertEquals(leaked, result.bytesOnDisk)
// The reconciler unlinks through the same deferred file system as everything else.
liveFs.drainNow()
assertTrue(
"directory must come back under budget (was $leaked, now ${bytesOnDisk()})",
bytesOnDisk() <= budget,
)
} finally {
second.shutdown()
liveScope.cancel()
}
}
@Test
fun clearAlone_wouldNotHaveReclaimedThem() {
// Pins why the reconciler does not just call DiskCache.clear(): evictAll() walks
// lruEntries, which is exactly the set of files the journal knows about, so orphans
// survive it. If this ever starts failing, Coil learned to sweep and this class can go.
val deadProcessFs = DeferredDeleteFileSystem(FileSystem.SYSTEM, scope)
scope.cancel("inert drainer")
val budget = 16L * 1024
val first = newCache(deadProcessFs, budget)
repeat(40) { i -> writeEntry(first, "key$i", 1024) }
val deadline = System.currentTimeMillis() + 5_000
while (first.size > budget && System.currentTimeMillis() < deadline) Thread.sleep(20)
first.shutdown()
val leaked = bytesOnDisk()
val liveScope = CoroutineScope(Dispatchers.IO + SupervisorJob())
val liveFs = DeferredDeleteFileSystem(FileSystem.SYSTEM, liveScope)
val second = newCache(liveFs, budget)
try {
second.clear()
liveFs.drainNow()
assertTrue(
"clear() should leave the orphans behind (was $leaked, now ${bytesOnDisk()})",
bytesOnDisk() > budget,
)
} finally {
second.shutdown()
liveScope.cancel()
}
}
@Test
fun aHealthyCacheIsLeftAlone() {
val fs = DeferredDeleteFileSystem(FileSystem.SYSTEM, scope)
val budget = 1024L * 1024
val diskCache = newCache(fs, budget)
try {
repeat(10) { i -> writeEntry(diskCache, "key$i", 1024) }
val before = bytesOnDisk()
val result = ImageDiskCacheReconciler.reconcile(diskCache)
assertFalse(result.wasOverBudget)
assertEquals(0, result.reclaimedFiles)
assertEquals(before, bytesOnDisk())
assertNotNull("entries must still be readable", diskCache.openSnapshot("key9")?.also { it.close() })
} finally {
diskCache.shutdown()
}
}
@Test
fun driftWithinTheSlackIsLeftAlone() {
// Being a little over the budget is normal: eviction is async, and a write in flight has a
// dirty file on disk that the journal has not accounted for yet. Only real drift is wiped.
val fs = DeferredDeleteFileSystem(FileSystem.SYSTEM, scope)
val budget = 16L * 1024
val diskCache = newCache(fs, budget)
try {
repeat(18) { i -> writeEntry(diskCache, "key$i", 1024) }
val result = ImageDiskCacheReconciler.reconcile(diskCache, minSlackBytes = 8 * 1024)
assertFalse("18 KB against a 16 KB budget + 8 KB slack is not drift", result.wasOverBudget)
} finally {
diskCache.shutdown()
}
}
@Test
fun aMissingDirectoryIsANoOp() {
val fs = DeferredDeleteFileSystem(FileSystem.SYSTEM, scope)
val diskCache = newCache(fs, 1024L * 1024)
try {
FileSystem.SYSTEM.deleteRecursively(cacheDir)
val result = ImageDiskCacheReconciler.reconcile(diskCache)
assertEquals(0L, result.bytesOnDisk)
assertFalse(result.wasOverBudget)
} finally {
diskCache.shutdown()
}
}
@Test
fun theShippedCeilingIsBudgetPlusAQuarter() {
// Pins the production numbers the injectable overrides above bypass.
val oneGb = 1024L * 1024 * 1024
assertEquals(oneGb + oneGb / 4, ImageDiskCacheReconciler.ceilingBytes(oneGb))
// ...but never less than the 4 MiB floor, so a small budget on a full disk is not wiped
// over the journal plus a couple of in-flight writes.
assertEquals(1024L * 1024 + 4L * 1024 * 1024, ImageDiskCacheReconciler.ceilingBytes(1024L * 1024))
}
// ---- cadence ----------------------------------------------------------------------------
// The healthy pass is a readdir plus a stat per file, and AppModules.initiate() runs on every
// process start — including the WorkManager wake-ups that cold-start the whole graph. Drift
// accrues over process deaths, not startups, so the check is rate-limited.
private val markerPath get() = cacheDir / ".reconciled"
@Test
fun aCacheWithNoRecordedPassIsDue() {
val diskCache = newCache(DeferredDeleteFileSystem(FileSystem.SYSTEM, scope), 1024L * 1024)
try {
assertNotNull(ImageDiskCacheReconciler.reconcileIfDue(diskCache))
assertNotNull("the pass must record itself", FileSystem.SYSTEM.metadataOrNull(markerPath))
} finally {
diskCache.shutdown()
}
}
@Test
fun aSecondStartWithinTheIntervalSkipsTheWalk() {
val fs = DeferredDeleteFileSystem(FileSystem.SYSTEM, scope)
val budget = 16L * 1024
val diskCache = newCache(fs, budget)
try {
val now = System.currentTimeMillis()
assertNotNull(ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = now))
// Even a directory well over budget is left alone until the next pass is due — the
// point of the gate is that the common start does no work at all.
repeat(40) { i -> writeEntry(diskCache, "key$i", 1024) }
val skipped = ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = now + 1000, minSlackBytes = 0)
assertNull("within the interval the pass must not run", skipped)
// Coil's size, not the directory's bytes: eviction runs asynchronously on its own scope
// and the drainer unlinks behind it, so comparing byte totals across the call races
// both. A wipe is what this needs to rule out, and clear() takes the size to zero.
assertTrue("a skipped pass must not have cleared the cache", diskCache.size > 0)
} finally {
diskCache.shutdown()
}
}
@Test
fun theIntervalIsMeasuredFromTheRecordedPass() {
val fs = DeferredDeleteFileSystem(FileSystem.SYSTEM, scope)
val diskCache = newCache(fs, 1024L * 1024)
try {
val interval = 24L * 60 * 60 * 1000
ImageDiskCacheReconciler.reconcileIfDue(diskCache, intervalMs = interval)
// Pin the recorded pass to a whole second before measuring from it. isDue() compares
// against the marker's mtime, and a file system that keeps mtime at whole-second
// resolution reads it back up to a second before the write that made it — so a boundary
// measured from our own clock instead lands a second early there. That is what made the
// first version of this test pass on a dev box and fail on CI.
val lastPass = (System.currentTimeMillis() / 1000) * 1000
assertTrue("could not set the marker's mtime", File(markerPath.toString()).setLastModified(lastPass))
assertEquals("the file system must keep the timestamp we set", lastPass, recordedPassAt())
assertNull(
"a millisecond before the interval is up, the pass must not run",
ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = lastPass + interval - 1, intervalMs = interval),
)
assertNotNull(
"one interval after the recorded pass, it is due again",
ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = lastPass + interval, intervalMs = interval),
)
} finally {
diskCache.shutdown()
}
}
private fun recordedPassAt(): Long =
requireNotNull(FileSystem.SYSTEM.metadataOrNull(markerPath)?.lastModifiedAtMillis) {
"a pass must record a timestamp"
}
@Test
fun aMarkerDatedInTheFutureDoesNotParkTheCheck() {
// A clock that jumped back, or a restored backup, would otherwise strand the check until
// real time caught up with the marker.
val fs = DeferredDeleteFileSystem(FileSystem.SYSTEM, scope)
val diskCache = newCache(fs, 1024L * 1024)
try {
val now = System.currentTimeMillis()
ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = now)
assertNotNull(ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = now - 365L * 24 * 60 * 60 * 1000))
} finally {
diskCache.shutdown()
}
}
@Test
fun theMarkerSurvivesAWipe() {
// The marker is a plain file in the cache directory, so the sweep would take it unless it is
// preserved explicitly — and a wipe that erases its own record makes every later start look
// due and walk again. Drives reconcile() rather than reconcileIfDue(), because the latter
// rewrites the marker afterwards and would mask the deletion.
val deadProcessFs = DeferredDeleteFileSystem(FileSystem.SYSTEM, scope)
scope.cancel("inert drainer")
val budget = 16L * 1024
val first = newCache(deadProcessFs, budget)
repeat(40) { i -> writeEntry(first, "key$i", 1024) }
val deadline = System.currentTimeMillis() + 5_000
while (first.size > budget && System.currentTimeMillis() < deadline) Thread.sleep(20)
first.shutdown()
val liveScope = CoroutineScope(Dispatchers.IO + SupervisorJob())
val liveFs = DeferredDeleteFileSystem(FileSystem.SYSTEM, liveScope)
val second = newCache(liveFs, budget)
try {
// Lay down a marker the way a previous pass would have.
FileSystem.SYSTEM.write(markerPath) {}
val result = ImageDiskCacheReconciler.reconcile(second, minSlackBytes = 0)
assertTrue("expected a wipe", result.wasOverBudget)
liveFs.drainNow()
assertNotNull("the marker must outlive the wipe", FileSystem.SYSTEM.metadataOrNull(markerPath))
} finally {
second.shutdown()
liveScope.cancel()
}
}
}
@@ -0,0 +1,231 @@
/*
* Copyright (c) 2025 Vitor Pamplona
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of
* this software and associated documentation files (the "Software"), to deal in
* the Software without restriction, including without limitation the rights to use,
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
* Software, and to permit persons to whom the Software is furnished to do so,
* subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/
package com.vitorpamplona.amethyst.service.images
import coil3.ColorImage
import coil3.decode.DataSource
import coil3.decode.ImageSource
import coil3.fetch.FetchResult
import coil3.fetch.Fetcher
import coil3.fetch.ImageFetchResult
import coil3.fetch.SourceFetchResult
import com.vitorpamplona.amethyst.commons.service.image.DeferredDeleteFileSystem
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel
import kotlinx.coroutines.test.runTest
import okio.Buffer
import okio.FileSystem
import okio.ForwardingFileSystem
import okio.Path
import okio.Path.Companion.toOkioPath
import org.junit.After
import org.junit.Assert.assertArrayEquals
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assert.assertSame
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import java.io.File
import java.nio.file.Files
/**
* Coil hands the platform `ImageDecoder` a file only when the source's file system is
* *referentially* `FileSystem.SYSTEM` (`ImageSource.toImageDecoderSourceOrNull`). Our disk cache
* wraps it in a [DeferredDeleteFileSystem], so without this fetcher every network image loses
* that path — silently: the decode still succeeds, it just copies the encoded image into RAM
* first (animated) or drops to `BitmapFactoryDecoder` (static). These tests pin the property that
* identity check reads, and the ownership transfer that keeps the disk-cache snapshot alive
* exactly as long as the source that replaces it.
*/
class SystemFileSystemFetcherTest {
private lateinit var tmpRoot: File
private lateinit var file: Path
private lateinit var scope: CoroutineScope
private lateinit var deferredDelete: DeferredDeleteFileSystem
private val bytes = ByteArray(64) { it.toByte() }
@Before
fun setUp() {
tmpRoot = Files.createTempDirectory("system-file-system-fetcher-test").toFile()
file = tmpRoot.resolve("blob.gif").toOkioPath()
FileSystem.SYSTEM.write(file) { write(bytes) }
scope = CoroutineScope(Dispatchers.IO + SupervisorJob())
deferredDelete = DeferredDeleteFileSystem(FileSystem.SYSTEM, scope)
}
@After
fun tearDown() {
scope.cancel()
tmpRoot.deleteRecursively()
}
private class FakeFetcher(
private val result: FetchResult?,
) : Fetcher {
override suspend fun fetch() = result
}
/** Records whether the disk-cache snapshot Coil would hand us has been released. */
private class Snapshot : AutoCloseable {
var closed = false
private set
override fun close() {
closed = true
}
}
private fun diskCacheSource(snapshot: AutoCloseable? = null) =
ImageSource(
file = file,
fileSystem = deferredDelete,
diskCacheKey = "https://example.com/blob.gif",
closeable = snapshot,
)
@Test
fun aDiskCacheSourceIsHandedOnTheSystemFileSystem() =
runTest {
val fetcher =
FakeFetcher(SourceFetchResult(diskCacheSource(), "image/gif", DataSource.DISK))
.onSystemFileSystem("https://example.com/blob.gif")
val result = fetcher.fetch() as SourceFetchResult
assertSame(
"Coil only reaches ImageDecoder when this is referentially FileSystem.SYSTEM",
FileSystem.SYSTEM,
result.source.fileSystem,
)
assertSame(file, result.source.fileOrNull())
assertArrayEquals(bytes, result.source.source().readByteArray())
result.source.close()
}
@Test
fun theRestOfTheResultSurvivesTheSwap() =
runTest {
val fetcher =
FakeFetcher(SourceFetchResult(diskCacheSource(), "image/gif", DataSource.NETWORK))
.onSystemFileSystem("https://example.com/blob.gif")
val result = fetcher.fetch() as SourceFetchResult
assertEquals("image/gif", result.mimeType)
assertEquals(DataSource.NETWORK, result.dataSource)
}
@Test
fun closingTheReplacementReleasesTheDiskCacheSnapshot() =
runTest {
// The engine closes exactly one source per fetch, and after the swap that is the
// replacement. If it did not own the original, the snapshot would leak and the cache
// entry would stay open forever.
val snapshot = Snapshot()
val fetcher =
FakeFetcher(SourceFetchResult(diskCacheSource(snapshot), null, DataSource.DISK))
.onSystemFileSystem("https://example.com/blob.gif")
val result = fetcher.fetch() as SourceFetchResult
assertFalse(snapshot.closed)
result.source.close()
assertTrue("closing the re-homed source must release the snapshot", snapshot.closed)
}
@Test
fun theDiskCacheKeyIsCarriedOver() =
runTest {
// FileImageSource.diskCacheKey is internal to Coil, but the engine reads it off the
// source to populate SuccessResult.diskCacheKey. Rebuilding the source without it
// would drop that silently, so reach for the field directly.
val fetcher =
FakeFetcher(SourceFetchResult(diskCacheSource(), null, DataSource.DISK))
.onSystemFileSystem("https://example.com/blob.gif")
val result = fetcher.fetch() as SourceFetchResult
val field =
result.source.javaClass.declaredFields
.firstOrNull { it.name == "diskCacheKey" }
assertNotNull("Coil renamed FileImageSource.diskCacheKey; re-check the re-home", field)
field!!.isAccessible = true
assertEquals("https://example.com/blob.gif", field.get(result.source))
}
@Test
fun aSourceAlreadyOnTheSystemFileSystemIsLeftAlone() =
runTest {
val original = SourceFetchResult(ImageSource(file, FileSystem.SYSTEM), null, DataSource.DISK)
val fetcher = FakeFetcher(original).onSystemFileSystem("key")
assertSame(original, fetcher.fetch())
}
@Test
fun anUnknownFileSystemWrapperIsLeftAlone() =
runTest {
// Only DeferredDeleteFileSystem is known to forward reads verbatim. Anything else may
// rewrite paths or serve different bytes, so the file underneath is not ours to hand out.
val original =
SourceFetchResult(
ImageSource(file, object : ForwardingFileSystem(FileSystem.SYSTEM) {}),
null,
DataSource.DISK,
)
val fetcher = FakeFetcher(original).onSystemFileSystem("key")
assertSame(original, fetcher.fetch())
}
@Test
fun aStreamBackedSourceIsNotMaterialisedIntoATempFile() =
runTest {
val source = ImageSource(Buffer().write(bytes), deferredDelete)
val original = SourceFetchResult(source, null, DataSource.NETWORK)
val fetcher = FakeFetcher(original).onSystemFileSystem("key")
assertSame(original, fetcher.fetch())
assertNull("re-homing must not force a temp copy of a stream", source.fileOrNull())
}
@Test
fun aNonSourceResultPassesStraightThrough() =
runTest {
// ProfilePictureFetcher answers thumbnail hits with an ImageFetchResult.
val original = ImageFetchResult(ColorImage(0), isSampled = true, DataSource.DISK)
assertSame(original, FakeFetcher(original).onSystemFileSystem("key").fetch())
}
@Test
fun aFetcherThatDeclinesStillDeclines() =
runTest {
assertNull(FakeFetcher(null).onSystemFileSystem("key").fetch())
}
}