diff --git a/app/src/main/java/eu/darken/capod/common/debug/recording/core/DebugSessionManager.kt b/app/src/main/java/eu/darken/capod/common/debug/recording/core/DebugSessionManager.kt index cdeb18df..b1699211 100644 --- a/app/src/main/java/eu/darken/capod/common/debug/recording/core/DebugSessionManager.kt +++ b/app/src/main/java/eu/darken/capod/common/debug/recording/core/DebugSessionManager.kt @@ -15,7 +15,6 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.combine -import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.update import kotlinx.coroutines.CancellationException @@ -43,7 +42,7 @@ class DebugSessionManager @Inject constructor( private val zippingIds = MutableStateFlow>(emptySet()) private val failedZipIds = MutableStateFlow>(emptySet()) private val refreshTrigger = MutableSharedFlow(extraBufferCapacity = 1) - private val pendingAutoZips = mutableSetOf() + private val pendingAutoZips: MutableSet = java.util.Collections.synchronizedSet(mutableSetOf()) val recorderState: Flow get() = recorderModule.state @@ -160,10 +159,11 @@ class DebugSessionManager @Inject constructor( refreshTrigger.tryEmit(Unit) } + private fun activeSessionId(): String? = recorderModule.currentLogDir?.let { deriveSessionId(it) } + suspend fun zipSession(sessionId: String): File = fsMutex.withLock { - val activeRecording = sessions.first().filterIsInstance() - .firstOrNull { it.id == sessionId } - require(activeRecording == null) { "Cannot zip an active recording session" } + // Do NOT call sessions.first() here — deadlock risk with fsMutex + require(activeSessionId() != sessionId) { "Cannot zip an active recording session" } val (dir, existingZip) = findSessionFiles(sessionId) @@ -185,15 +185,19 @@ class DebugSessionManager @Inject constructor( } suspend fun deleteSession(sessionId: String) = fsMutex.withLock { - val currentSessions = sessions.first() - val recording = currentSessions.filterIsInstance() - .firstOrNull { it.id == sessionId } - require(recording == null) { "Cannot delete an active recording session" } + // Do NOT call sessions.first() here — deadlock risk with fsMutex + require(activeSessionId() != sessionId) { "Cannot delete an active recording session" } require(sessionId !in zippingIds.value) { "Cannot delete a session that is being compressed" } - val (dir, zip) = findSessionFiles(sessionId) - dir?.deleteRecursively() - zip?.delete() + withContext(dispatcherProvider.IO) { + val (dir, zip) = findSessionFiles(sessionId) + if (dir?.deleteRecursively() == false) { + log(TAG, WARN) { "Failed to fully delete session dir: ${dir.path}" } + } + if (zip?.delete() == false) { + log(TAG, WARN) { "Failed to delete session zip: ${zip.path}" } + } + } failedZipIds.update { it - sessionId } log(TAG) { "Deleted session: $sessionId" } @@ -201,24 +205,23 @@ class DebugSessionManager @Inject constructor( } suspend fun deleteAllSessions() = fsMutex.withLock { - val activeDir = recorderModule.state.first().currentLogDir + val activeDir = recorderModule.currentLogDir val currentlyZipping = zippingIds.value - for (dir in recorderModule.getLogDirectories()) { - if (!dir.exists()) continue - for (entry in dir.listFiles() ?: emptyArray()) { - if (entry == activeDir) { - log(TAG) { "Skipping active session dir: $entry" } - continue - } - val entryId = deriveSessionId(entry) - if (entryId in currentlyZipping) { - log(TAG) { "Skipping zipping session: $entry" } - continue - } - if (entry.isDirectory) { - entry.deleteRecursively() - } else { - entry.delete() + withContext(dispatcherProvider.IO) { + for (dir in recorderModule.getLogDirectories()) { + if (!dir.exists()) continue + for (entry in dir.listFiles() ?: emptyArray()) { + if (entry == activeDir) { + log(TAG) { "Skipping active session dir: $entry" } + continue + } + val entryId = deriveSessionId(entry) + if (entryId in currentlyZipping) { + log(TAG) { "Skipping zipping session: $entry" } + continue + } + val deleted = if (entry.isDirectory) entry.deleteRecursively() else entry.delete() + if (!deleted) log(TAG, WARN) { "Failed to delete: ${entry.path}" } } } } diff --git a/app/src/main/java/eu/darken/capod/common/debug/recording/core/RecorderModule.kt b/app/src/main/java/eu/darken/capod/common/debug/recording/core/RecorderModule.kt index a37f8e2d..d806d460 100644 --- a/app/src/main/java/eu/darken/capod/common/debug/recording/core/RecorderModule.kt +++ b/app/src/main/java/eu/darken/capod/common/debug/recording/core/RecorderModule.kt @@ -33,6 +33,10 @@ class RecorderModule @Inject constructor( private val installId: InstallId, ) { + @Volatile + internal var currentLogDir: File? = null + private set + private val triggerFile = try { File(context.getExternalFilesDir(null), FORCE_FILE) } catch (e: Exception) { @@ -64,6 +68,8 @@ class RecorderModule @Inject constructor( log(TAG, INFO) { "Build.Fingerprint: ${Build.FINGERPRINT}" } log(TAG, INFO) { "BuildConfig.Versions: ${BuildConfigWrap.VERSION_DESCRIPTION}" } + this@RecorderModule.currentLogDir = sessionDir + copy( recorder = newRecorder, currentLogDir = sessionDir, @@ -76,6 +82,8 @@ class RecorderModule @Inject constructor( log(TAG, ERROR) { "Failed to delete trigger file" } } + this@RecorderModule.currentLogDir = null + copy( recorder = null, currentLogDir = null, diff --git a/app/src/test/java/eu/darken/capod/common/debug/recording/core/DebugSessionManagerTest.kt b/app/src/test/java/eu/darken/capod/common/debug/recording/core/DebugSessionManagerTest.kt index 205e6f72..498b9e90 100644 --- a/app/src/test/java/eu/darken/capod/common/debug/recording/core/DebugSessionManagerTest.kt +++ b/app/src/test/java/eu/darken/capod/common/debug/recording/core/DebugSessionManagerTest.kt @@ -199,6 +199,23 @@ class DebugSessionManagerTest : BaseTest() { // Verify the session IS a Recording — the manager would reject zip/delete calls for this } + @Test + fun `ext and cache sessions with same basename have different IDs`() { + val cacheLogsDir = File(tempDir, "cache/debug/logs").also { it.mkdirs() } + val extDir = File(externalLogsDir, "capod_1.0_1700000000000_abcd1234").also { it.mkdirs() } + File(extDir, "core.log").writeText("external log") + val cacheDir = File(cacheLogsDir, "capod_1.0_1700000000000_abcd1234").also { it.mkdirs() } + File(cacheDir, "core.log").writeText("cache log") + + val sessions = DebugSessionManager.scanSessions( + logDirectories = listOf(externalLogsDir, cacheLogsDir), + ) + + sessions shouldHaveSize 2 + val ids = sessions.map { it.id }.toSet() + ids shouldBe setOf("ext:capod_1.0_1700000000000_abcd1234", "cache:capod_1.0_1700000000000_abcd1234") + } + @Test fun `zipping session should not be deleted`() { val sessionDir = File(externalLogsDir, "capod_1.0_1700000000000_abcd1234").also { it.mkdirs() }