From cba65344b9128ef85084dcfc94e1fccac4710d78 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 24 Mar 2026 04:37:41 +0000 Subject: [PATCH 1/5] fix: clean up temp files after upload completes MetadataStripper, MediaCompressor, and EncryptFiles create intermediate temp files in cacheDir during the upload pipeline, but these were never deleted after the upload finished. Over time this causes unbounded disk growth. Wrap upload calls in try/finally to ensure all intermediate temp files (compressed, stripped, encrypted) are deleted regardless of success or failure. https://claude.ai/code/session_01YS3dbZ1k84N2EHS3AruYRV --- .../service/uploads/UploadOrchestrator.kt | 52 ++++++++++++++++--- 1 file changed, 44 insertions(+), 8 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/UploadOrchestrator.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/UploadOrchestrator.kt index ee5ef4e728..0c54b765ee 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/UploadOrchestrator.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/UploadOrchestrator.kt @@ -31,10 +31,12 @@ import com.vitorpamplona.amethyst.service.uploads.nip96.Nip96Uploader import com.vitorpamplona.amethyst.ui.actions.mediaServers.ServerName import com.vitorpamplona.amethyst.ui.actions.mediaServers.ServerType import com.vitorpamplona.quartz.nip01Core.signers.SignerExceptions +import com.vitorpamplona.quartz.utils.Log import com.vitorpamplona.quartz.utils.ciphers.NostrCipher import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.map import okhttp3.OkHttpClient +import java.io.File import kotlin.coroutines.cancellation.CancellationException sealed class UploadingState { @@ -324,6 +326,26 @@ class UploadOrchestrator { return strippingResult.uri } + /** + * Deletes a temporary file created during the upload pipeline if its URI + * differs from the original (meaning it's an intermediate temp file, not the user's content). + */ + private fun deleteTempUri( + tempUri: Uri, + originalUri: Uri, + ) { + if (tempUri == originalUri) return + try { + val path = tempUri.path ?: return + val file = File(path) + if (file.exists() && file.delete()) { + Log.d("UploadOrchestrator", "Deleted temp file: $path") + } + } catch (e: Exception) { + Log.w("UploadOrchestrator", "Failed to delete temp file: ${tempUri.path}", e) + } + } + suspend fun upload( uri: Uri, mimeType: String?, @@ -343,10 +365,17 @@ class UploadOrchestrator { stripAfterCompression(uri, compressed, mimeType, compressionQuality, stripMetadata, onStrippingFailed, context) ?: return error(R.string.upload_cancelled) - return when (server.type) { - ServerType.NIP95 -> uploadNIP95(finalUri, compressed.contentType, null, null, context) - ServerType.NIP96 -> uploadNIP96(finalUri, compressed.contentType, compressed.size, alt, contentWarningReason, server.baseUrl, null, null, account, context) - ServerType.Blossom -> uploadBlossom(finalUri, compressed.contentType, compressed.size, alt, contentWarningReason, server.baseUrl, null, null, account, context) + try { + return when (server.type) { + ServerType.NIP95 -> uploadNIP95(finalUri, compressed.contentType, null, null, context) + ServerType.NIP96 -> uploadNIP96(finalUri, compressed.contentType, compressed.size, alt, contentWarningReason, server.baseUrl, null, null, account, context) + ServerType.Blossom -> uploadBlossom(finalUri, compressed.contentType, compressed.size, alt, contentWarningReason, server.baseUrl, null, null, account, context) + } + } finally { + // Clean up intermediate temp files created by stripping and compression. + // Delete stripped file first (if different from compressed), then compressed (if different from original). + deleteTempUri(finalUri, compressed.uri) + deleteTempUri(compressed.uri, uri) } } @@ -372,10 +401,17 @@ class UploadOrchestrator { val encrypted = EncryptFiles().encryptFile(context, finalUri, encrypt) - return when (server.type) { - ServerType.NIP95 -> uploadNIP95(encrypted.uri, encrypted.contentType, compressed.contentType, encrypted.originalHash, context) - ServerType.NIP96 -> uploadNIP96(encrypted.uri, encrypted.contentType, encrypted.size, alt, contentWarningReason, server.baseUrl, compressed.contentType, encrypted.originalHash, account, context) - ServerType.Blossom -> uploadBlossom(encrypted.uri, encrypted.contentType, encrypted.size, alt, contentWarningReason, server.baseUrl, compressed.contentType, encrypted.originalHash, account, context) + try { + return when (server.type) { + ServerType.NIP95 -> uploadNIP95(encrypted.uri, encrypted.contentType, compressed.contentType, encrypted.originalHash, context) + ServerType.NIP96 -> uploadNIP96(encrypted.uri, encrypted.contentType, encrypted.size, alt, contentWarningReason, server.baseUrl, compressed.contentType, encrypted.originalHash, account, context) + ServerType.Blossom -> uploadBlossom(encrypted.uri, encrypted.contentType, encrypted.size, alt, contentWarningReason, server.baseUrl, compressed.contentType, encrypted.originalHash, account, context) + } + } finally { + // Clean up all intermediate temp files: encrypted, stripped, and compressed. + deleteTempUri(encrypted.uri, finalUri) + deleteTempUri(finalUri, compressed.uri) + deleteTempUri(compressed.uri, uri) } } } From 390edf311d548fd56d2ddde1d103bd34c752688f Mon Sep 17 00:00:00 2001 From: davotoula Date: Wed, 25 Mar 2026 09:29:48 +0100 Subject: [PATCH 2/5] analysis for temporary files --- docs/temp-file-cleanup-analysis.md | 109 +++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 docs/temp-file-cleanup-analysis.md diff --git a/docs/temp-file-cleanup-analysis.md b/docs/temp-file-cleanup-analysis.md new file mode 100644 index 0000000000..25299b935b --- /dev/null +++ b/docs/temp-file-cleanup-analysis.md @@ -0,0 +1,109 @@ +# Temporary File Cleanup Analysis + +## Overview + +Analysis of temporary file creation and cleanup patterns across the Amethyst codebase. +The goal is to identify opportunities for more aggressive cleanup — deleting temp files +as soon as they are no longer needed rather than deferring to cache cleanup. + +## Temporary File Creation Sites (Android) + +| Area | File | Creates | Cleanup | Notes | +|------|------|---------|---------|-------| +| Image compression | `MediaCompressor.kt:94` | Temp copy via `MediaCompressorFileUtils.from()` | UploadOrchestrator `finally` | Deferred to cache cleanup | +| Image metadata strip | `MetadataStripper.kt:199` | `stripped_*.jpg` in cacheDir | UploadOrchestrator `finally` | Deferred to cache cleanup | +| Video metadata strip | `MetadataStripper.kt:238` | `stripped_video_*.mp4` in cacheDir | UploadOrchestrator `finally` | Deferred to cache cleanup | +| Audio metadata strip | `MetadataStripper.kt:286` | `stripped_audio_*.m4a` in cacheDir | UploadOrchestrator `finally` | Deferred to cache cleanup | +| MP3 metadata strip | `MetadataStripper.kt:307,363` | `mp3_input_*` + `stripped_mp3_*` | Mixed: some immediate, some orchestrator | MP3 input file cleaned inline | +| File encryption | `EncryptFiles.kt:47` | `EncryptFiles*.encrypted` in cacheDir | UploadOrchestrator `finally` | Deferred to cache cleanup | +| URI temp copy | `MediaCompressorFileUtils.kt:41` | Random UUID temp file | Caller-dependent | No self-cleanup | +| Voice anonymization | `VoiceAnonymizationController.kt:85` | Distorted voice files | `deleteDistortedFiles()` explicit | Cleaned by controller | +| Video sharing | `ZoomableContentView.kt:905` | Temp video + sharable copy | Delayed GlobalScope (1 min) | Intentional delay for sharing | +| Camera capture | `TakePicture.kt:244` | Camera temp file | System/caller | Managed by camera provider | + +## Temporary File Creation Sites (Desktop) + +| Area | File | Creates | Cleanup | Notes | +|------|------|---------|---------|-------| +| Clipboard paste | `ClipboardPasteHandler.kt:43` | `clipboard_*.png` | `deleteOnExit()` only | Leaks until JVM exit | +| Image compression | `DesktopMediaCompressor.kt:42` | `stripped_*.jpg` | `deleteOnExit()` only | Leaks until JVM exit | + +## The Upload Pipeline + +The `UploadOrchestrator` is the central cleanup coordinator. Its `finally` blocks call +`deleteTempUri()` to delete intermediate files after upload completes. The pipeline stages: + +``` +1. MediaCompressorFileUtils.from() --> temp copy of original URI +2. MediaCompressor.compress() --> compressed file (may replace #1) +3. MetadataStripper.strip*() --> stripped file (new temp) +4. (optional) EncryptFiles.encrypt() --> encrypted file (new temp) +5. Upload to server +6. finally: delete all intermediates via deleteTempUri() +``` + +Each stage may produce a new temp file. Currently, all intermediates accumulate and are +only cleaned up in the `finally` block after the upload succeeds or fails. + +## Cleanup Patterns in Use + +### 1. Chained Deletion (UploadOrchestrator) +- Most common pattern +- `finally` blocks at `L374-379` (upload) and `L410-415` (uploadEncrypted) +- Deletes all intermediate temp files after pipeline completes +- Checks `tempUri != originalUri` to avoid deleting user's original file + +### 2. Delayed Deletion (Video Sharing) +- `ZoomableContentView.kt:953-960` +- Uses `GlobalScope.launch(Dispatchers.IO)` with 1 minute delay +- Intentional: allows recipient app time to read the shared file +- **Exception: this delay is necessary and should not be shortened** + +### 3. Explicit Cleanup Calls (Voice) +- `VoiceAnonymizationController.deleteDistortedFiles()` at `L108-123` +- Called explicitly by `ShortNotePostViewModel` at `L1336` +- Good pattern: controller owns its temp files + +### 4. deleteOnExit() (Desktop) +- Used by `ClipboardPasteHandler` and `DesktopMediaCompressor` +- Files persist until JVM process exits +- Not ideal for long-running desktop app + +## Opportunities for More Aggressive Cleanup + +### 1. Pipeline Stage Cleanup (UploadOrchestrator) + +**Current:** All intermediates deleted in `finally` after upload. + +**Proposed:** Delete each intermediate as soon as the next stage produces output. + +Example: after `MetadataStripper` produces a stripped file, delete the compressed file +from the previous stage immediately, rather than keeping it alive through upload + finally. + +### 2. Desktop Temp Files + +**Current:** `deleteOnExit()` — files persist for entire app session. + +**Proposed:** Delete immediately after upload completes, matching the Android +`UploadOrchestrator` pattern via `DesktopUploadOrchestrator`. + +### 3. MediaCompressorFileUtils Intermediate + +**Current:** `from()` creates a temp copy at `L41`. If compression produces a *different* +file, the original temp copy is orphaned until `UploadOrchestrator.finally`. + +**Proposed:** `MediaCompressor` should delete the `from()` temp file immediately after +compression succeeds (when the compressed file differs from the input). + +### 4. Voice Anonymization Intermediates + +**Current:** `deleteDistortedFiles()` called after upload. + +**Proposed:** Already reasonably aggressive. Could delete each intermediate distortion +result as soon as the next processing step completes, if there are multiple stages. + +## Exception: Video Sharing + +Sharing a video to other Android apps requires the temporary file to remain accessible +for at least 1 minute. The current `SHARED_VIDEO_CLEANUP_DELAY_MS` delay in +`ZoomableContentView.kt` handles this correctly and should **not** be made more aggressive. \ No newline at end of file From 60b7c6bd0d778aaa8ce75e37b47dfea3f31eb012 Mon Sep 17 00:00:00 2001 From: davotoula Date: Wed, 25 Mar 2026 10:04:44 +0100 Subject: [PATCH 3/5] fix: delete abandoned compressed video when larger than original fix: eagerly delete intermediate temp files in upload pipeline fix: delete temp file from MediaCompressorFileUtils after image compression refactor: simplify temp file cleanup logic - Remove TOCTOU anti-pattern (file.exists() before file.delete()) - Consolidate double deleteTempUri calls into single conditional - Remove restating comments --- .../service/uploads/MediaCompressor.kt | 13 +++++++--- .../service/uploads/UploadOrchestrator.kt | 25 +++++++++++-------- .../service/uploads/VideoCompressionHelper.kt | 1 + 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/MediaCompressor.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/MediaCompressor.kt index f984e72886..875dd72d3f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/MediaCompressor.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/MediaCompressor.kt @@ -31,6 +31,7 @@ import com.vitorpamplona.quartz.utils.Log import id.zelory.compressor.Compressor import id.zelory.compressor.constraint.default import kotlinx.coroutines.CancellationException +import java.io.File class MediaCompressorResult( val uri: Uri, @@ -89,19 +90,23 @@ class MediaCompressor { else -> 60 } + var tempFile: File? = null return try { Log.d("MediaCompressor", "Using image compression $mediaQuality") - val tempFile = MediaCompressorFileUtils.from(uri, context) + tempFile = MediaCompressorFileUtils.from(uri, context) val compressedImageFile = Compressor.compress(context, tempFile) { default(width = 640, format = Bitmap.CompressFormat.JPEG, quality = imageQuality) } - Log.d("MediaCompressor", "Image compression success. Original size [${tempFile.length()}], new size [${compressedImageFile.length()}]") + if (tempFile != compressedImageFile && !tempFile.delete()) { + Log.w("MediaCompressor", "Failed to delete temp file: ${tempFile.absolutePath}") + } + Log.d("MediaCompressor", "Image compression success. New size [${compressedImageFile.length()}]") MediaCompressorResult(compressedImageFile.toUri(), MimeTypes.IMAGE_JPEG, compressedImageFile.length()) } catch (e: Exception) { - Log.d("MediaCompressor", "Image compression failed: ${e.message}") if (e is CancellationException) throw e - e.printStackTrace() + Log.d("MediaCompressor", "Image compression failed: ${e.message}") + tempFile?.delete() MediaCompressorResult(uri, contentType, null) } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/UploadOrchestrator.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/UploadOrchestrator.kt index 0c54b765ee..b56d78932f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/UploadOrchestrator.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/UploadOrchestrator.kt @@ -338,7 +338,7 @@ class UploadOrchestrator { try { val path = tempUri.path ?: return val file = File(path) - if (file.exists() && file.delete()) { + if (file.delete()) { Log.d("UploadOrchestrator", "Deleted temp file: $path") } } catch (e: Exception) { @@ -363,7 +363,11 @@ class UploadOrchestrator { val finalUri = stripAfterCompression(uri, compressed, mimeType, compressionQuality, stripMetadata, onStrippingFailed, context) - ?: return error(R.string.upload_cancelled) + ?: return error(R.string.upload_cancelled).also { + deleteTempUri(compressed.uri, uri) + } + + if (compressed.uri != finalUri) deleteTempUri(compressed.uri, uri) try { return when (server.type) { @@ -372,10 +376,7 @@ class UploadOrchestrator { ServerType.Blossom -> uploadBlossom(finalUri, compressed.contentType, compressed.size, alt, contentWarningReason, server.baseUrl, null, null, account, context) } } finally { - // Clean up intermediate temp files created by stripping and compression. - // Delete stripped file first (if different from compressed), then compressed (if different from original). - deleteTempUri(finalUri, compressed.uri) - deleteTempUri(compressed.uri, uri) + deleteTempUri(finalUri, uri) } } @@ -397,9 +398,14 @@ class UploadOrchestrator { val finalUri = stripAfterCompression(uri, compressed, mimeType, compressionQuality, stripMetadata, onStrippingFailed, context) - ?: return error(R.string.upload_cancelled) + ?: return error(R.string.upload_cancelled).also { + deleteTempUri(compressed.uri, uri) + } + + if (compressed.uri != finalUri) deleteTempUri(compressed.uri, uri) val encrypted = EncryptFiles().encryptFile(context, finalUri, encrypt) + deleteTempUri(finalUri, uri) try { return when (server.type) { @@ -408,10 +414,7 @@ class UploadOrchestrator { ServerType.Blossom -> uploadBlossom(encrypted.uri, encrypted.contentType, encrypted.size, alt, contentWarningReason, server.baseUrl, compressed.contentType, encrypted.originalHash, account, context) } } finally { - // Clean up all intermediate temp files: encrypted, stripped, and compressed. - deleteTempUri(encrypted.uri, finalUri) - deleteTempUri(finalUri, compressed.uri) - deleteTempUri(compressed.uri, uri) + deleteTempUri(encrypted.uri, uri) } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/VideoCompressionHelper.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/VideoCompressionHelper.kt index 8f98f6fb9a..3d3c64715e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/VideoCompressionHelper.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/uploads/VideoCompressionHelper.kt @@ -233,6 +233,7 @@ object VideoCompressionHelper { // Sanity check: compression not smaller than original if (originalSize > 0 && size >= originalSize) { + File(path).delete() applicationContext.notifyUser( "Compressed file larger than original. Using original.", Log.WARN, From 42db03790a157e777b1e9e474a6edb9e62db45d7 Mon Sep 17 00:00:00 2001 From: davotoula Date: Wed, 25 Mar 2026 10:23:38 +0100 Subject: [PATCH 4/5] added progress and test plan to doc --- docs/temp-file-cleanup-analysis.md | 234 ++++++++++++++++++++--------- 1 file changed, 163 insertions(+), 71 deletions(-) diff --git a/docs/temp-file-cleanup-analysis.md b/docs/temp-file-cleanup-analysis.md index 25299b935b..d9acd1ea55 100644 --- a/docs/temp-file-cleanup-analysis.md +++ b/docs/temp-file-cleanup-analysis.md @@ -8,18 +8,19 @@ as soon as they are no longer needed rather than deferring to cache cleanup. ## Temporary File Creation Sites (Android) -| Area | File | Creates | Cleanup | Notes | -|------|------|---------|---------|-------| -| Image compression | `MediaCompressor.kt:94` | Temp copy via `MediaCompressorFileUtils.from()` | UploadOrchestrator `finally` | Deferred to cache cleanup | -| Image metadata strip | `MetadataStripper.kt:199` | `stripped_*.jpg` in cacheDir | UploadOrchestrator `finally` | Deferred to cache cleanup | -| Video metadata strip | `MetadataStripper.kt:238` | `stripped_video_*.mp4` in cacheDir | UploadOrchestrator `finally` | Deferred to cache cleanup | -| Audio metadata strip | `MetadataStripper.kt:286` | `stripped_audio_*.m4a` in cacheDir | UploadOrchestrator `finally` | Deferred to cache cleanup | -| MP3 metadata strip | `MetadataStripper.kt:307,363` | `mp3_input_*` + `stripped_mp3_*` | Mixed: some immediate, some orchestrator | MP3 input file cleaned inline | -| File encryption | `EncryptFiles.kt:47` | `EncryptFiles*.encrypted` in cacheDir | UploadOrchestrator `finally` | Deferred to cache cleanup | -| URI temp copy | `MediaCompressorFileUtils.kt:41` | Random UUID temp file | Caller-dependent | No self-cleanup | -| Voice anonymization | `VoiceAnonymizationController.kt:85` | Distorted voice files | `deleteDistortedFiles()` explicit | Cleaned by controller | -| Video sharing | `ZoomableContentView.kt:905` | Temp video + sharable copy | Delayed GlobalScope (1 min) | Intentional delay for sharing | -| Camera capture | `TakePicture.kt:244` | Camera temp file | System/caller | Managed by camera provider | +| Area | File | Creates | Cleanup | Status | +|------|------|---------|---------|--------| +| Image compression | `MediaCompressor.kt:94` | Temp copy via `MediaCompressorFileUtils.from()` | Deleted immediately after compression | FIXED | +| Image metadata strip | `MetadataStripper.kt:199` | `stripped_*.jpg` in cacheDir | Deleted eagerly by orchestrator | FIXED | +| Video metadata strip | `MetadataStripper.kt:238` | `stripped_video_*.mp4` in cacheDir | Deleted eagerly by orchestrator | FIXED | +| Audio metadata strip | `MetadataStripper.kt:286` | `stripped_audio_*.m4a` in cacheDir | Deleted eagerly by orchestrator | FIXED | +| MP3 metadata strip | `MetadataStripper.kt:307,363` | `mp3_input_*` + `stripped_mp3_*` | Mixed: some immediate, some orchestrator | Already OK | +| File encryption | `EncryptFiles.kt:47` | `EncryptFiles*.encrypted` in cacheDir | Deleted eagerly by orchestrator | FIXED | +| URI temp copy | `MediaCompressorFileUtils.kt:41` | Random UUID temp file | Deleted by MediaCompressor after use | FIXED | +| Video compression | `VideoCompressionHelper.kt` | Compressed video in app storage | Abandoned file deleted when larger than original | FIXED | +| Voice anonymization | `VoiceAnonymizationController.kt:85` | Distorted voice files | `deleteDistortedFiles()` explicit | Already OK | +| Video sharing | `ZoomableContentView.kt:905` | Temp video + sharable copy | Delayed GlobalScope (1 min) | Skipped (intentional) | +| Camera capture | `TakePicture.kt:244` | Camera temp file | System/caller | Skipped (system-managed) | ## Temporary File Creation Sites (Desktop) @@ -30,80 +31,171 @@ as soon as they are no longer needed rather than deferring to cache cleanup. ## The Upload Pipeline -The `UploadOrchestrator` is the central cleanup coordinator. Its `finally` blocks call -`deleteTempUri()` to delete intermediate files after upload completes. The pipeline stages: +The `UploadOrchestrator` is the central cleanup coordinator. Each intermediate temp file +is now deleted as soon as the next pipeline stage produces its output: ``` -1. MediaCompressorFileUtils.from() --> temp copy of original URI -2. MediaCompressor.compress() --> compressed file (may replace #1) -3. MetadataStripper.strip*() --> stripped file (new temp) -4. (optional) EncryptFiles.encrypt() --> encrypted file (new temp) +1. MediaCompressorFileUtils.from() --> temp copy of original URI +2. MediaCompressor.compress() --> compressed file; temp copy from #1 deleted immediately +3. MetadataStripper.strip*() --> stripped file; compressed file from #2 deleted immediately +4. (optional) EncryptFiles.encrypt() --> encrypted file; stripped file from #3 deleted immediately 5. Upload to server -6. finally: delete all intermediates via deleteTempUri() +6. finally: delete the last remaining intermediate ``` -Each stage may produce a new temp file. Currently, all intermediates accumulate and are -only cleaned up in the `finally` block after the upload succeeds or fails. +## What Was Fixed -## Cleanup Patterns in Use +### 1. MediaCompressor temp file leak (MediaCompressor.kt) +- `MediaCompressorFileUtils.from()` created a temp copy that was never deleted +- Now deleted immediately after `Compressor.compress()` produces a separate output file +- Also cleaned up on compression failure (catch block) -### 1. Chained Deletion (UploadOrchestrator) -- Most common pattern -- `finally` blocks at `L374-379` (upload) and `L410-415` (uploadEncrypted) -- Deletes all intermediate temp files after pipeline completes -- Checks `tempUri != originalUri` to avoid deleting user's original file +### 2. Eager pipeline cleanup (UploadOrchestrator.kt) +- `upload()`: compressed file deleted right after stripping produces `finalUri` +- `uploadEncrypted()`: compressed file deleted after stripping, stripped file deleted + after encryption — only the encrypted file survives until after upload +- Cancel path also cleans up compressed file via `.also {}` block +- `finally` block now only handles the last surviving intermediate -### 2. Delayed Deletion (Video Sharing) -- `ZoomableContentView.kt:953-960` -- Uses `GlobalScope.launch(Dispatchers.IO)` with 1 minute delay -- Intentional: allows recipient app time to read the shared file -- **Exception: this delay is necessary and should not be shortened** +### 3. Abandoned compressed video (VideoCompressionHelper.kt) +- When compressed video is larger than original, the original is used instead +- The abandoned compressed file was leaked — now deleted before returning -### 3. Explicit Cleanup Calls (Voice) -- `VoiceAnonymizationController.deleteDistortedFiles()` at `L108-123` -- Called explicitly by `ShortNotePostViewModel` at `L1336` -- Good pattern: controller owns its temp files +## What Was Skipped -### 4. deleteOnExit() (Desktop) -- Used by `ClipboardPasteHandler` and `DesktopMediaCompressor` -- Files persist until JVM process exits -- Not ideal for long-running desktop app +### Desktop temp files (out of scope for this change) +- `ClipboardPasteHandler.kt` and `DesktopMediaCompressor.kt` use `deleteOnExit()` +- Files persist until JVM process exits — not ideal for a long-running desktop app +- `DesktopUploadOrchestrator.kt` uses bare `processedFile.delete()` with no error + handling or logging, diverging from the Android `deleteTempUri` pattern +- **Reason:** User requested Android-only focus for this iteration -## Opportunities for More Aggressive Cleanup +### Voice anonymization intermediates +- `VoiceAnonymizationController.deleteDistortedFiles()` is already reasonably aggressive +- Called explicitly by `ShortNotePostViewModel` after upload +- **Reason:** Already working well, no leak identified -### 1. Pipeline Stage Cleanup (UploadOrchestrator) +### Video sharing delay +- `ZoomableContentView.kt` uses a 1-minute delay before cleanup +- **Reason:** Intentional — receiving app needs time to read the shared file -**Current:** All intermediates deleted in `finally` after upload. - -**Proposed:** Delete each intermediate as soon as the next stage produces output. - -Example: after `MetadataStripper` produces a stripped file, delete the compressed file -from the previous stage immediately, rather than keeping it alive through upload + finally. - -### 2. Desktop Temp Files - -**Current:** `deleteOnExit()` — files persist for entire app session. - -**Proposed:** Delete immediately after upload completes, matching the Android -`UploadOrchestrator` pattern via `DesktopUploadOrchestrator`. - -### 3. MediaCompressorFileUtils Intermediate - -**Current:** `from()` creates a temp copy at `L41`. If compression produces a *different* -file, the original temp copy is orphaned until `UploadOrchestrator.finally`. - -**Proposed:** `MediaCompressor` should delete the `from()` temp file immediately after -compression succeeds (when the compressed file differs from the input). - -### 4. Voice Anonymization Intermediates - -**Current:** `deleteDistortedFiles()` called after upload. - -**Proposed:** Already reasonably aggressive. Could delete each intermediate distortion -result as soon as the next processing step completes, if there are multiple stages. +### Camera capture temp files +- `TakePicture.kt` creates temp files via camera provider +- **Reason:** Managed by the Android system/camera provider, not our responsibility ## Exception: Video Sharing Sharing a video to other Android apps requires the temporary file to remain accessible for at least 1 minute. The current `SHARED_VIDEO_CLEANUP_DELAY_MS` delay in -`ZoomableContentView.kt` handles this correctly and should **not** be made more aggressive. \ No newline at end of file +`ZoomableContentView.kt` handles this correctly and should **not** be made more aggressive. + +## Manual Test Plan + +### Setup + +Enable `adb logcat` filtering to observe cleanup behavior: + +```bash +adb logcat -s MediaCompressor:* UploadOrchestrator:* VideoCompressionHelper:* MetadataStripper:* +``` + +To verify temp files are actually being deleted, monitor the cache directory before and +after each test: + +```bash +adb shell "ls -la /data/data/com.vitorpamplona.amethyst/cache/ | grep -E 'stripped_|EncryptFiles|mp3_input|stripped_mp3|stripped_video|stripped_audio'" +``` + +### Test 1: Image upload with compression + +1. Open a new note compose screen +2. Attach a JPEG photo from the gallery +3. Set compression quality to Medium +4. Post the note +5. **Verify in logcat:** + - `MediaCompressor: Image compression success` appears + - `MediaCompressor: Failed to delete temp file` does NOT appear + - `UploadOrchestrator: Deleted temp file` appears (for the stripped file after upload) +6. **Verify in cache dir:** No `stripped_*.jpg` files remain after upload completes + +### Test 2: Image upload without compression + +1. Open a new note compose screen +2. Attach a JPEG photo from the gallery +3. Set compression quality to Uncompressed +4. Post the note +5. **Verify in logcat:** + - No `MediaCompressor` compression log appears + - `UploadOrchestrator: Deleted temp file` appears for the stripped file +6. **Verify in cache dir:** No `stripped_*` files remain + +### Test 3: Video upload with compression + +1. Open a new note compose screen +2. Attach a video from the gallery +3. Set compression quality to Medium +4. Post the note +5. **Verify in logcat:** + - `VideoCompressionHelper: Compression success` appears + - `UploadOrchestrator: Deleted temp file` appears +6. **Verify in cache dir:** No `stripped_video_*.mp4` files remain + +### Test 4: Video compression produces larger file + +1. Attach a very small or already-compressed video +2. Set compression to Low quality +3. Post the note +4. **Verify in logcat:** + - `VideoCompressionHelper: Compressed file larger than original. Using original.` appears + - The compressed file is deleted (no orphaned file in cache) + +### Test 5: Audio/voice message upload + +1. Record a voice message in a note or reply +2. Send it +3. **Verify in logcat:** + - `UploadOrchestrator: Deleted temp file` appears for the stripped audio +4. **Verify in cache dir:** No `stripped_audio_*.m4a` files remain + +### Test 6: MP3 upload + +1. Attach an MP3 file (with ID3 tags) from the file picker +2. Post the note +3. **Verify in logcat:** + - `MetadataStripper: Stripped ID3 tags from MP3` appears + - `UploadOrchestrator: Deleted temp file` appears +4. **Verify in cache dir:** No `mp3_input_*` or `stripped_mp3_*` files remain + +### Test 7: Encrypted file upload (NIP-44 DM) + +1. Open a DM conversation +2. Attach an image +3. Send the message (triggers encrypted upload path) +4. **Verify in logcat:** + - Compressed file is deleted after stripping + - Stripped file is deleted after encryption + - Encrypted file is deleted after upload + - Three separate `UploadOrchestrator: Deleted temp file` log lines appear +5. **Verify in cache dir:** No `stripped_*` or `EncryptFiles*` files remain + +### Test 8: Upload cancellation + +1. Open a new note compose screen +2. Attach a large image or video +3. Cancel the upload while compression or upload is in progress +4. **Verify in cache dir:** No temp files remain from the cancelled upload + +### Test 9: Video sharing to other apps + +1. Open a note with a video +2. Long-press or use the share button to share the video to another app +3. **Verify:** The receiving app successfully receives the video +4. **Verify:** After ~1 minute, the temp file in the share directory is cleaned up +5. **This test confirms the 1-minute delay was not broken by our changes** + +### Test 10: Image compression failure fallback + +1. Attach a GIF or SVG file (compression is skipped for these) +2. Post the note +3. **Verify:** Upload succeeds using the original file +4. **Verify in cache dir:** No orphaned temp files \ No newline at end of file From aa5dad9fac85193f65bf85fe89b9b7c53abd370b Mon Sep 17 00:00:00 2001 From: davotoula Date: Wed, 25 Mar 2026 17:42:05 +0100 Subject: [PATCH 5/5] delete voice files on failuer --- .../ui/screen/loggedIn/home/ShortNotePostViewModel.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/ShortNotePostViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/ShortNotePostViewModel.kt index 9b41af0b59..fc68d621d9 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/ShortNotePostViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/ShortNotePostViewModel.kt @@ -697,6 +697,8 @@ open class ShortNotePostViewModel : // Abort if upload failed - don't post without voice data if (voiceMetadata == null) { Log.w("ShortNotePostViewModel", "Voice upload failed, aborting post") + deleteVoiceLocalFile() + voiceAnonymization.deleteDistortedFiles() return } // Update default server if voice message was successfully uploaded @@ -1274,8 +1276,7 @@ open class ShortNotePostViewModel : private fun deleteVoiceLocalFile() { voiceLocalFile?.let { file -> try { - if (file.exists()) { - file.delete() + if (file.delete()) { Log.d("ShortNotePostViewModel", "Deleted voice file: ${file.absolutePath}") } } catch (e: Exception) {