From 3d544a8e34d30d830bf540a7d2f08e742ebbaa57 Mon Sep 17 00:00:00 2001 From: darken Date: Tue, 15 Oct 2024 13:24:31 +0200 Subject: [PATCH 1/3] Fix SD Maid creating large cache files that are not being deleted when creating video previews When SD Maid generates thumbnails for large videos via coil, then large `.tmp` files can be created. If SD Maid is killed before thumbnail generation has finished, then these `.tmp` can stay around until manually deleted. This PR changes the path where these files are stored, and deleted any orphaned files `.tmp` files on the next app launch. Also see https://github.com/coil-kt/coil/issues/2550 --- app/src/main/java/eu/darken/sdmse/App.kt | 4 ++ .../darken/sdmse/common/coil/BitmapFetcher.kt | 9 ++- .../darken/sdmse/common/coil/CoilTempFiles.kt | 64 +++++++++++++++++++ .../sdmse/common/coil/PathPreviewFetcher.kt | 17 ++++- .../sdmse/common/coil/CoilTempFilesTest.kt | 16 +++++ 5 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 app/src/main/java/eu/darken/sdmse/common/coil/CoilTempFiles.kt create mode 100644 app/src/test/java/eu/darken/sdmse/common/coil/CoilTempFilesTest.kt diff --git a/app/src/main/java/eu/darken/sdmse/App.kt b/app/src/main/java/eu/darken/sdmse/App.kt index b42e23f12..c39d3544c 100644 --- a/app/src/main/java/eu/darken/sdmse/App.kt +++ b/app/src/main/java/eu/darken/sdmse/App.kt @@ -8,6 +8,7 @@ import coil.ImageLoaderFactory import dagger.hilt.android.HiltAndroidApp import eu.darken.sdmse.common.BuildConfigWrap import eu.darken.sdmse.common.BuildWrap +import eu.darken.sdmse.common.coil.CoilTempFiles import eu.darken.sdmse.common.coroutine.AppScope import eu.darken.sdmse.common.coroutine.DispatcherProvider import eu.darken.sdmse.common.debug.AutomaticBugReporter @@ -28,6 +29,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.launch import javax.inject.Inject import kotlin.system.exitProcess @@ -45,6 +47,7 @@ open class App : Application(), Configuration.Provider { @Inject lateinit var curriculumVitae: CurriculumVitae @Inject lateinit var updateService: UpdateService @Inject lateinit var theming: Theming + @Inject lateinit var coilTempFiles: CoilTempFiles private val logCatLogger = LogCatLogger() @@ -83,6 +86,7 @@ open class App : Application(), Configuration.Provider { theming.setup() + appScope.launch { coilTempFiles.cleanUp() } Coil.setImageLoader(imageLoaderFactory) curriculumVitae.updateAppLaunch() diff --git a/app/src/main/java/eu/darken/sdmse/common/coil/BitmapFetcher.kt b/app/src/main/java/eu/darken/sdmse/common/coil/BitmapFetcher.kt index e707583f4..df3f8a62e 100644 --- a/app/src/main/java/eu/darken/sdmse/common/coil/BitmapFetcher.kt +++ b/app/src/main/java/eu/darken/sdmse/common/coil/BitmapFetcher.kt @@ -18,6 +18,7 @@ import javax.inject.Inject class BitmapFetcher @Inject constructor( @ApplicationContext private val context: Context, + private val coilTempFiles: CoilTempFiles, private val gatewaySwitch: GatewaySwitch, private val mimeTypeTool: MimeTypeTool, private val data: Request, @@ -38,7 +39,10 @@ class BitmapFetcher @Inject constructor( val buffer = gatewaySwitch.read(target.lookedUp).buffer() return SourceResult( - ImageSource(buffer, context), + ImageSource( + buffer, + coilTempFiles.getBaseCachePath(), + ), mimeType, dataSource = DataSource.DISK ) @@ -46,6 +50,7 @@ class BitmapFetcher @Inject constructor( class Factory @Inject constructor( @ApplicationContext private val context: Context, + private val coilTempFiles: CoilTempFiles, private val gatewaySwitch: GatewaySwitch, private val mimeTypeTool: MimeTypeTool, ) : Fetcher.Factory { @@ -54,7 +59,7 @@ class BitmapFetcher @Inject constructor( data: Request, options: Options, imageLoader: ImageLoader - ): Fetcher = BitmapFetcher(context, gatewaySwitch, mimeTypeTool, data, options) + ): Fetcher = BitmapFetcher(context, coilTempFiles, gatewaySwitch, mimeTypeTool, data, options) } data class Request( diff --git a/app/src/main/java/eu/darken/sdmse/common/coil/CoilTempFiles.kt b/app/src/main/java/eu/darken/sdmse/common/coil/CoilTempFiles.kt new file mode 100644 index 000000000..c9595dee4 --- /dev/null +++ b/app/src/main/java/eu/darken/sdmse/common/coil/CoilTempFiles.kt @@ -0,0 +1,64 @@ +package eu.darken.sdmse.common.coil + +import android.content.Context +import dagger.hilt.android.qualifiers.ApplicationContext +import eu.darken.sdmse.common.coroutine.DispatcherProvider +import eu.darken.sdmse.common.debug.logging.Logging.Priority.VERBOSE +import eu.darken.sdmse.common.debug.logging.Logging.Priority.WARN +import eu.darken.sdmse.common.debug.logging.asLog +import eu.darken.sdmse.common.debug.logging.log +import eu.darken.sdmse.common.debug.logging.logTag +import eu.darken.sdmse.common.files.core.local.listFiles2 +import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.withContext +import java.io.File +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +class CoilTempFiles @Inject constructor( + @ApplicationContext private val context: Context, + private val dispatcherProvider: DispatcherProvider, +) { + + private val basePath: File = File(context.cacheDir, "coil") + private val legacyPath: File = context.cacheDir + + suspend fun getBaseCachePath(): File { + val path = legacyPath.apply { + if (mkdirs()) log(TAG) { "Cache path was created: $this" } + } + return path + } + + suspend fun cleanUp() = withContext(dispatcherProvider.IO + NonCancellable) { + if (!basePath.exists()) return@withContext + + try { + log(TAG) { "Checking for legacy files in $legacyPath" } + legacyPath.listFiles2() + .filter { NAME_REGEX.matches(it.name) } + .forEach { + log(TAG) { "Deleting legacy tmp file: $it (${it.length()} Byte)" } + if (it.delete()) log(TAG, VERBOSE) { "Deleted legacy file: $it" } + } + } catch (e: Exception) { + log(TAG, WARN) { "Failed to clean up legacy files $basePath:\n${e.asLog()}" } + } + + try { + log(TAG) { "Cleaning up $basePath" } + basePath.listFiles2().forEach { + log(TAG) { "Deleting orphaned tmp file: $it (${it.length()} Byte)" } + if (it.delete()) log(TAG, VERBOSE) { "Deleted: $it" } + } + } catch (e: Exception) { + log(TAG, WARN) { "Failed to clean up $basePath:\n${e.asLog()}" } + } + } + + companion object { + private val TAG = logTag("Coil", "TempFiles") + internal val NAME_REGEX = Regex("""tmp\d+\.tmp""") + } +} \ No newline at end of file diff --git a/app/src/main/java/eu/darken/sdmse/common/coil/PathPreviewFetcher.kt b/app/src/main/java/eu/darken/sdmse/common/coil/PathPreviewFetcher.kt index 68aac671d..26089d1cb 100644 --- a/app/src/main/java/eu/darken/sdmse/common/coil/PathPreviewFetcher.kt +++ b/app/src/main/java/eu/darken/sdmse/common/coil/PathPreviewFetcher.kt @@ -27,6 +27,7 @@ import javax.inject.Inject class PathPreviewFetcher @Inject constructor( @ApplicationContext private val context: Context, + private val coilTempFiles: CoilTempFiles, private val generalSettings: GeneralSettings, private val gatewaySwitch: GatewaySwitch, private val mimeTypeTool: MimeTypeTool, @@ -56,7 +57,10 @@ class PathPreviewFetcher @Inject constructor( mimeType.startsWith("image") || mimeType.startsWith("video") -> { val buffer = gatewaySwitch.read(data.lookedUp).buffer() SourceResult( - ImageSource(buffer, context), + ImageSource( + buffer, + coilTempFiles.getBaseCachePath(), + ), mimeType, dataSource = DataSource.DISK ) @@ -91,6 +95,7 @@ class PathPreviewFetcher @Inject constructor( class Factory @Inject constructor( @ApplicationContext private val context: Context, + private val coilTempFiles: CoilTempFiles, private val generalSettings: GeneralSettings, private val gatewaySwitch: GatewaySwitch, private val mimeTypeTool: MimeTypeTool, @@ -100,7 +105,15 @@ class PathPreviewFetcher @Inject constructor( data: APathLookup<*>, options: Options, imageLoader: ImageLoader - ): Fetcher = PathPreviewFetcher(context, generalSettings, gatewaySwitch, mimeTypeTool, data, options) + ): Fetcher = PathPreviewFetcher( + context, + coilTempFiles, + generalSettings, + gatewaySwitch, + mimeTypeTool, + data, + options, + ) } } diff --git a/app/src/test/java/eu/darken/sdmse/common/coil/CoilTempFilesTest.kt b/app/src/test/java/eu/darken/sdmse/common/coil/CoilTempFilesTest.kt new file mode 100644 index 000000000..f83fa9da1 --- /dev/null +++ b/app/src/test/java/eu/darken/sdmse/common/coil/CoilTempFilesTest.kt @@ -0,0 +1,16 @@ +package eu.darken.sdmse.common.coil + +import io.kotest.matchers.shouldBe +import org.junit.jupiter.api.Test +import testhelpers.BaseTest + + +class CoilTempFilesTest : BaseTest() { + + @Test fun `legacy cleanup tmp file path matching`() { + CoilTempFiles.NAME_REGEX.matches("tmp9050482114132890229.tmp") shouldBe true + CoilTempFiles.NAME_REGEX.matches("9050482114132890229.tmp") shouldBe false + CoilTempFiles.NAME_REGEX.matches("tmp9050482114132890229.other") shouldBe false + CoilTempFiles.NAME_REGEX.matches("tmp90504229.tmp") shouldBe true + } +} \ No newline at end of file From 7132301be803fca404c81d0a9780f83d3b0b7e29 Mon Sep 17 00:00:00 2001 From: darken Date: Tue, 15 Oct 2024 13:24:31 +0200 Subject: [PATCH 2/3] Fix SD Maid large cache files not being deleted when creating video previews When SD Maid generates thumbnails for large videos via coil, then large `.tmp` files can be created. If SD Maid is killed before thumbnail generation has finished, then these `.tmp` can stay around until manually deleted. This PR changes the path where these files are stored, and deleted any orphaned files `.tmp` files on the next app launch. Also see https://github.com/coil-kt/coil/issues/2550 --- app/src/main/java/eu/darken/sdmse/App.kt | 4 ++ .../darken/sdmse/common/coil/BitmapFetcher.kt | 9 ++- .../darken/sdmse/common/coil/CoilTempFiles.kt | 64 +++++++++++++++++++ .../sdmse/common/coil/PathPreviewFetcher.kt | 17 ++++- .../sdmse/common/coil/CoilTempFilesTest.kt | 16 +++++ 5 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 app/src/main/java/eu/darken/sdmse/common/coil/CoilTempFiles.kt create mode 100644 app/src/test/java/eu/darken/sdmse/common/coil/CoilTempFilesTest.kt diff --git a/app/src/main/java/eu/darken/sdmse/App.kt b/app/src/main/java/eu/darken/sdmse/App.kt index b42e23f12..c39d3544c 100644 --- a/app/src/main/java/eu/darken/sdmse/App.kt +++ b/app/src/main/java/eu/darken/sdmse/App.kt @@ -8,6 +8,7 @@ import coil.ImageLoaderFactory import dagger.hilt.android.HiltAndroidApp import eu.darken.sdmse.common.BuildConfigWrap import eu.darken.sdmse.common.BuildWrap +import eu.darken.sdmse.common.coil.CoilTempFiles import eu.darken.sdmse.common.coroutine.AppScope import eu.darken.sdmse.common.coroutine.DispatcherProvider import eu.darken.sdmse.common.debug.AutomaticBugReporter @@ -28,6 +29,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.launch import javax.inject.Inject import kotlin.system.exitProcess @@ -45,6 +47,7 @@ open class App : Application(), Configuration.Provider { @Inject lateinit var curriculumVitae: CurriculumVitae @Inject lateinit var updateService: UpdateService @Inject lateinit var theming: Theming + @Inject lateinit var coilTempFiles: CoilTempFiles private val logCatLogger = LogCatLogger() @@ -83,6 +86,7 @@ open class App : Application(), Configuration.Provider { theming.setup() + appScope.launch { coilTempFiles.cleanUp() } Coil.setImageLoader(imageLoaderFactory) curriculumVitae.updateAppLaunch() diff --git a/app/src/main/java/eu/darken/sdmse/common/coil/BitmapFetcher.kt b/app/src/main/java/eu/darken/sdmse/common/coil/BitmapFetcher.kt index e707583f4..df3f8a62e 100644 --- a/app/src/main/java/eu/darken/sdmse/common/coil/BitmapFetcher.kt +++ b/app/src/main/java/eu/darken/sdmse/common/coil/BitmapFetcher.kt @@ -18,6 +18,7 @@ import javax.inject.Inject class BitmapFetcher @Inject constructor( @ApplicationContext private val context: Context, + private val coilTempFiles: CoilTempFiles, private val gatewaySwitch: GatewaySwitch, private val mimeTypeTool: MimeTypeTool, private val data: Request, @@ -38,7 +39,10 @@ class BitmapFetcher @Inject constructor( val buffer = gatewaySwitch.read(target.lookedUp).buffer() return SourceResult( - ImageSource(buffer, context), + ImageSource( + buffer, + coilTempFiles.getBaseCachePath(), + ), mimeType, dataSource = DataSource.DISK ) @@ -46,6 +50,7 @@ class BitmapFetcher @Inject constructor( class Factory @Inject constructor( @ApplicationContext private val context: Context, + private val coilTempFiles: CoilTempFiles, private val gatewaySwitch: GatewaySwitch, private val mimeTypeTool: MimeTypeTool, ) : Fetcher.Factory { @@ -54,7 +59,7 @@ class BitmapFetcher @Inject constructor( data: Request, options: Options, imageLoader: ImageLoader - ): Fetcher = BitmapFetcher(context, gatewaySwitch, mimeTypeTool, data, options) + ): Fetcher = BitmapFetcher(context, coilTempFiles, gatewaySwitch, mimeTypeTool, data, options) } data class Request( diff --git a/app/src/main/java/eu/darken/sdmse/common/coil/CoilTempFiles.kt b/app/src/main/java/eu/darken/sdmse/common/coil/CoilTempFiles.kt new file mode 100644 index 000000000..15f1a461f --- /dev/null +++ b/app/src/main/java/eu/darken/sdmse/common/coil/CoilTempFiles.kt @@ -0,0 +1,64 @@ +package eu.darken.sdmse.common.coil + +import android.content.Context +import dagger.hilt.android.qualifiers.ApplicationContext +import eu.darken.sdmse.common.coroutine.DispatcherProvider +import eu.darken.sdmse.common.debug.logging.Logging.Priority.VERBOSE +import eu.darken.sdmse.common.debug.logging.Logging.Priority.WARN +import eu.darken.sdmse.common.debug.logging.asLog +import eu.darken.sdmse.common.debug.logging.log +import eu.darken.sdmse.common.debug.logging.logTag +import eu.darken.sdmse.common.files.core.local.listFiles2 +import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.withContext +import java.io.File +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +class CoilTempFiles @Inject constructor( + @ApplicationContext private val context: Context, + private val dispatcherProvider: DispatcherProvider, +) { + + private val basePath: File = File(context.cacheDir, "coil") + private val legacyPath: File = context.cacheDir + + suspend fun getBaseCachePath(): File { + val path = basePath.apply { + if (mkdirs()) log(TAG) { "Cache path was created: $this" } + } + return path + } + + suspend fun cleanUp() = withContext(dispatcherProvider.IO + NonCancellable) { + try { + log(TAG) { "Checking for legacy files in $legacyPath" } + legacyPath.listFiles2() + .filter { NAME_REGEX.matches(it.name) } + .forEach { + log(TAG) { "Deleting legacy tmp file: $it (${it.length()} Byte)" } + if (it.delete()) log(TAG, VERBOSE) { "Deleted legacy file: $it" } + } + } catch (e: Exception) { + log(TAG, WARN) { "Failed to clean up legacy files $basePath:\n${e.asLog()}" } + } + + if (!basePath.exists()) return@withContext + + try { + log(TAG) { "Cleaning up $basePath" } + basePath.listFiles2().forEach { + log(TAG) { "Deleting orphaned tmp file: $it (${it.length()} Byte)" } + if (it.delete()) log(TAG, VERBOSE) { "Deleted: $it" } + } + } catch (e: Exception) { + log(TAG, WARN) { "Failed to clean up $basePath:\n${e.asLog()}" } + } + } + + companion object { + private val TAG = logTag("Coil", "TempFiles") + internal val NAME_REGEX = Regex("""tmp\d+\.tmp""") + } +} \ No newline at end of file diff --git a/app/src/main/java/eu/darken/sdmse/common/coil/PathPreviewFetcher.kt b/app/src/main/java/eu/darken/sdmse/common/coil/PathPreviewFetcher.kt index 68aac671d..26089d1cb 100644 --- a/app/src/main/java/eu/darken/sdmse/common/coil/PathPreviewFetcher.kt +++ b/app/src/main/java/eu/darken/sdmse/common/coil/PathPreviewFetcher.kt @@ -27,6 +27,7 @@ import javax.inject.Inject class PathPreviewFetcher @Inject constructor( @ApplicationContext private val context: Context, + private val coilTempFiles: CoilTempFiles, private val generalSettings: GeneralSettings, private val gatewaySwitch: GatewaySwitch, private val mimeTypeTool: MimeTypeTool, @@ -56,7 +57,10 @@ class PathPreviewFetcher @Inject constructor( mimeType.startsWith("image") || mimeType.startsWith("video") -> { val buffer = gatewaySwitch.read(data.lookedUp).buffer() SourceResult( - ImageSource(buffer, context), + ImageSource( + buffer, + coilTempFiles.getBaseCachePath(), + ), mimeType, dataSource = DataSource.DISK ) @@ -91,6 +95,7 @@ class PathPreviewFetcher @Inject constructor( class Factory @Inject constructor( @ApplicationContext private val context: Context, + private val coilTempFiles: CoilTempFiles, private val generalSettings: GeneralSettings, private val gatewaySwitch: GatewaySwitch, private val mimeTypeTool: MimeTypeTool, @@ -100,7 +105,15 @@ class PathPreviewFetcher @Inject constructor( data: APathLookup<*>, options: Options, imageLoader: ImageLoader - ): Fetcher = PathPreviewFetcher(context, generalSettings, gatewaySwitch, mimeTypeTool, data, options) + ): Fetcher = PathPreviewFetcher( + context, + coilTempFiles, + generalSettings, + gatewaySwitch, + mimeTypeTool, + data, + options, + ) } } diff --git a/app/src/test/java/eu/darken/sdmse/common/coil/CoilTempFilesTest.kt b/app/src/test/java/eu/darken/sdmse/common/coil/CoilTempFilesTest.kt new file mode 100644 index 000000000..f83fa9da1 --- /dev/null +++ b/app/src/test/java/eu/darken/sdmse/common/coil/CoilTempFilesTest.kt @@ -0,0 +1,16 @@ +package eu.darken.sdmse.common.coil + +import io.kotest.matchers.shouldBe +import org.junit.jupiter.api.Test +import testhelpers.BaseTest + + +class CoilTempFilesTest : BaseTest() { + + @Test fun `legacy cleanup tmp file path matching`() { + CoilTempFiles.NAME_REGEX.matches("tmp9050482114132890229.tmp") shouldBe true + CoilTempFiles.NAME_REGEX.matches("9050482114132890229.tmp") shouldBe false + CoilTempFiles.NAME_REGEX.matches("tmp9050482114132890229.other") shouldBe false + CoilTempFiles.NAME_REGEX.matches("tmp90504229.tmp") shouldBe true + } +} \ No newline at end of file From 1a217fdd080a6e3c82ccf49ddfe21c8563fb833f Mon Sep 17 00:00:00 2001 From: darken Date: Tue, 15 Oct 2024 13:26:55 +0200 Subject: [PATCH 3/3] Less parallelism when loading video thumbnails, otherwise loading 10 previews for videos could consume huge amounts of space at once --- app/src/main/java/eu/darken/sdmse/common/coil/CoilModule.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/eu/darken/sdmse/common/coil/CoilModule.kt b/app/src/main/java/eu/darken/sdmse/common/coil/CoilModule.kt index 4a389cfc6..bbed678d1 100644 --- a/app/src/main/java/eu/darken/sdmse/common/coil/CoilModule.kt +++ b/app/src/main/java/eu/darken/sdmse/common/coil/CoilModule.kt @@ -52,7 +52,7 @@ class CoilModule { } dispatcher( dispatcherProvider.Default.limitedParallelism( - (Runtime.getRuntime().availableProcessors() - 1).coerceAtLeast(2) + (Runtime.getRuntime().availableProcessors() / 2).coerceAtLeast(2) ) ) }.build()