Fix thread starvation caused by not yielding or using an inappropriate thread pool (#2955)

CHANGELOG.md

@@ -22,6 +22,7 @@ The format is a modified version of [Keep a Changelog](https://keepachangelog.co
 - Minimize memory usage by reducing in-memory cover cache size ([@Lolle2000la](https://github.com/Lolle2000la)) ([#2266](https://github.com/mihonapp/mihon/pull/2266))
 - Optimize MAL search queries ([@MajorTanya](https://github.com/MajorTanya)) ([#2832](https://github.com/mihonapp/mihon/pull/2832))
 - Reword download reindexing message to avoid confusion ([@MajorTanya](https://github.com/MajorTanya)) ([#2874](https://github.com/mihonapp/mihon/pull/2874))
+- Rework internals for better performance ([@Lolle2000la](https://github.com/Lolle2000la)) ([#2955](https://github.com/mihonapp/mihon/pull/2955))
 
 ### Changed
 - Update tracker icons ([@AntsyLich](https://github.com/AntsyLich)) ([#2773](https://github.com/mihonapp/mihon/pull/2773))

app/src/main/java/eu/kanade/presentation/more/settings/screen/SettingsDataScreen.kt

@@ -285,7 +285,10 @@ object SettingsDataScreen : SearchableSettings {
 
         val chapterCache = remember { Injekt.get<ChapterCache>() }
         var cacheReadableSizeSema by remember { mutableIntStateOf(0) }
-        val cacheReadableSize = remember(cacheReadableSizeSema) { chapterCache.readableSize }
+        var cacheReadableSize by remember { mutableStateOf(context.stringResource(MR.strings.calculating)) }
+        LaunchedEffect(cacheReadableSizeSema) {
+            cacheReadableSize = chapterCache.getReadableSize()
+        }
 
         return Preference.PreferenceGroup(
             title = stringResource(MR.strings.pref_storage_usage),

app/src/main/java/eu/kanade/presentation/more/settings/screen/data/StorageInfo.kt

@@ -9,12 +9,18 @@ import androidx.compose.material3.LinearProgressIndicator
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.LaunchedEffect
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
+import androidx.compose.runtime.setValue
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.clip
 import androidx.compose.ui.platform.LocalContext
 import androidx.compose.ui.unit.dp
 import eu.kanade.tachiyomi.util.storage.DiskUtil
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.withContext
 import tachiyomi.i18n.MR
 import tachiyomi.presentation.core.components.material.padding
 import tachiyomi.presentation.core.i18n.stringResource
@@ -45,10 +51,24 @@ private fun StorageInfo(
 ) {
     val context = LocalContext.current
 
-    val available = remember(file) { DiskUtil.getAvailableStorageSpace(file) }
+    var available by remember(file) { mutableStateOf(-1L) }
-    val availableText = remember(available) { Formatter.formatFileSize(context, available) }
+    var total by remember(file) { mutableStateOf(-1L) }
-    val total = remember(file) { DiskUtil.getTotalStorageSpace(file) }
+
-    val totalText = remember(total) { Formatter.formatFileSize(context, total) }
+    LaunchedEffect(file) {
+        available = withContext(Dispatchers.IO) { DiskUtil.getAvailableStorageSpace(file) }
+        total = withContext(Dispatchers.IO) { DiskUtil.getTotalStorageSpace(file) }
+    }
+
+    val availableText = if (available == -1L) {
+        stringResource(MR.strings.calculating)
+    } else {
+        Formatter.formatFileSize(context, available)
+    }
+    val totalText = if (total == -1L) {
+        stringResource(MR.strings.calculating)
+    } else {
+        Formatter.formatFileSize(context, total)
+    }
 
     Column(
         verticalArrangement = Arrangement.spacedBy(MaterialTheme.padding.extraSmall),
@@ -58,6 +78,7 @@ private fun StorageInfo(
             style = MaterialTheme.typography.header,
         )
 
+        if (total > 0) {
             LinearProgressIndicator(
                 modifier = Modifier
                     .clip(MaterialTheme.shapes.small)
@@ -65,6 +86,7 @@ private fun StorageInfo(
                     .height(12.dp),
                 progress = { (1 - (available / total.toFloat())) },
             )
+        }
 
         Text(
             text = stringResource(MR.strings.available_disk_space_info, availableText, totalText),

app/src/main/java/eu/kanade/tachiyomi/data/cache/ChapterCache.kt

@@ -6,6 +6,8 @@ import com.jakewharton.disklrucache.DiskLruCache
 import eu.kanade.tachiyomi.source.model.Page
 import eu.kanade.tachiyomi.util.storage.DiskUtil
 import eu.kanade.tachiyomi.util.storage.saveTo
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.withContext
 import kotlinx.serialization.json.Json
 import logcat.LogPriority
 import okhttp3.Response
@@ -42,17 +44,13 @@ class ChapterCache(
      */
     private val cacheDir: File = diskCache.directory
 
-    /**
-     * Returns real size of directory.
-     */
-    private val realSize: Long
-        get() = DiskUtil.getDirectorySize(cacheDir)
-
     /**
      * Returns real size of directory in human readable format.
      */
-    val readableSize: String
+    suspend fun getReadableSize(): String = withContext(Dispatchers.IO) {
-        get() = Formatter.formatFileSize(context, realSize)
+        val size = DiskUtil.getDirectorySize(cacheDir)
+        Formatter.formatFileSize(context, size)
+    }
 
     /**
      * Get page list from cache.

app/src/main/java/eu/kanade/tachiyomi/data/download/DownloadManager.kt

@@ -104,10 +104,10 @@ class DownloadManager(
         return queueState.value.find { it.chapter.id == chapterId }
     }
 
-    fun startDownloadNow(chapterId: Long) {
+    suspend fun startDownloadNow(chapterId: Long) {
         val existingDownload = getQueuedDownloadOrNull(chapterId)
         // If not in queue try to start a new download
-        val toAdd = existingDownload ?: runBlocking { Download.fromChapterId(chapterId) } ?: return
+        val toAdd = existingDownload ?: Download.fromChapterId(chapterId) ?: return
         queueState.value.toMutableList().apply {
             existingDownload?.let { remove(it) }
             add(0, toAdd)

app/src/main/java/eu/kanade/tachiyomi/data/download/DownloadStore.kt

@@ -90,7 +90,7 @@ class DownloadStore(
     /**
      * Returns the list of downloads to restore. It should be called in a background thread.
      */
-    fun restore(): List<Download> {
+    suspend fun restore(): List<Download> {
         val objs = preferences.all
             .mapNotNull { it.value as? String }
             .mapNotNull { deserialize(it) }
@@ -101,10 +101,10 @@ class DownloadStore(
             val cachedManga = mutableMapOf<Long, Manga?>()
             for ((mangaId, chapterId) in objs) {
                 val manga = cachedManga.getOrPut(mangaId) {
-                    runBlocking { getManga.await(mangaId) }
+                    getManga.await(mangaId)
                 } ?: continue
                 val source = sourceManager.get(manga.source) as? HttpSource ?: continue
-                val chapter = runBlocking { getChapter.await(chapterId) } ?: continue
+                val chapter = getChapter.await(chapterId) ?: continue
                 downloads.add(Download(source, manga, chapter))
             }
         }

app/src/main/java/eu/kanade/tachiyomi/data/download/Downloader.kt

@@ -111,9 +111,9 @@ class Downloader(
     var isPaused: Boolean = false
 
     init {
-        launchNow {
+        scope.launch {
-            val chapters = async { store.restore() }
+            val chapters = store.restore()
-            addAllToQueue(chapters.await())
+            addAllToQueue(chapters)
         }
     }
 

app/src/main/java/eu/kanade/tachiyomi/data/notification/NotificationReceiver.kt

@@ -20,6 +20,7 @@ import eu.kanade.tachiyomi.util.system.toast
 import kotlinx.coroutines.runBlocking
 import tachiyomi.core.common.Constants
 import tachiyomi.core.common.util.lang.launchIO
+import tachiyomi.core.common.util.lang.withUIContext
 import tachiyomi.domain.chapter.interactor.GetChapter
 import tachiyomi.domain.chapter.interactor.UpdateChapter
 import tachiyomi.domain.chapter.model.Chapter
@@ -78,11 +79,18 @@ class NotificationReceiver : BroadcastReceiver() {
             ACTION_CANCEL_APP_UPDATE_DOWNLOAD -> cancelDownloadAppUpdate(context)
             // Open reader activity
             ACTION_OPEN_CHAPTER -> {
+                val pendingResult = goAsync()
+                launchIO {
+                    try {
                         openChapter(
                             context,
                             intent.getLongExtra(EXTRA_MANGA_ID, -1),
                             intent.getLongExtra(EXTRA_CHAPTER_ID, -1),
                         )
+                    } finally {
+                        pendingResult.finish()
+                    }
+                }
             }
             // Mark updated manga chapters as read
             ACTION_MARK_AS_READ -> {
@@ -147,9 +155,10 @@ class NotificationReceiver : BroadcastReceiver() {
      * @param mangaId id of manga
      * @param chapterId id of chapter
      */
-    private fun openChapter(context: Context, mangaId: Long, chapterId: Long) {
+    private suspend fun openChapter(context: Context, mangaId: Long, chapterId: Long) {
-        val manga = runBlocking { getManga.await(mangaId) }
+        val manga = getManga.await(mangaId)
-        val chapter = runBlocking { getChapter.await(chapterId) }
+        val chapter = getChapter.await(chapterId)
+        withUIContext {
             if (manga != null && chapter != null) {
                 val intent = ReaderActivity.newIntent(context, manga.id, chapter.id).apply {
                     flags = Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP
@@ -159,6 +168,7 @@ class NotificationReceiver : BroadcastReceiver() {
                 context.toast(MR.strings.chapter_error)
             }
         }
+    }
 
     /**
      * Method called when user wants to stop a backup restore job.

app/src/main/java/eu/kanade/tachiyomi/extension/ExtensionManager.kt

@@ -23,6 +23,7 @@ import kotlinx.coroutines.flow.asStateFlow
 import kotlinx.coroutines.flow.emptyFlow
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.stateIn
+import kotlinx.coroutines.launch
 import logcat.LogPriority
 import tachiyomi.core.common.util.lang.withUIContext
 import tachiyomi.core.common.util.system.logcat
@@ -118,6 +119,7 @@ class ExtensionManager(
      * Loads and registers the installed extensions.
      */
     private fun initExtensions() {
+        scope.launch {
             val extensions = ExtensionLoader.loadExtensions(context)
 
             installedExtensionMapFlow.value = extensions
@@ -130,6 +132,7 @@ class ExtensionManager(
 
             _isInitialized.value = true
         }
+    }
 
     /**
      * Finds the available extensions in the [api] and updates [availableExtensionMapFlow].

app/src/main/java/eu/kanade/tachiyomi/extension/util/ExtensionLoader.kt

@@ -18,6 +18,7 @@ import eu.kanade.tachiyomi.util.storage.copyAndSetReadOnlyTo
 import eu.kanade.tachiyomi.util.system.ChildFirstPathClassLoader
 import kotlinx.coroutines.async
 import kotlinx.coroutines.awaitAll
+import kotlinx.coroutines.coroutineScope
 import kotlinx.coroutines.runBlocking
 import logcat.LogPriority
 import tachiyomi.core.common.util.system.logcat
@@ -114,7 +115,7 @@ internal object ExtensionLoader {
      *
      * @param context The application context.
      */
-    fun loadExtensions(context: Context): List<LoadResult> {
+    suspend fun loadExtensions(context: Context): List<LoadResult> {
         val pkgManager = context.packageManager
 
         val installedPkgs = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
@@ -160,11 +161,10 @@ internal object ExtensionLoader {
         if (extPkgs.isEmpty()) return emptyList()
 
         // Load each extension concurrently and wait for completion
-        return runBlocking {
+        return coroutineScope {
-            val deferred = extPkgs.map {
+            extPkgs.map {
                 async { loadExtension(context, it) }
-            }
+            }.awaitAll()
-            deferred.awaitAll()
         }
     }
 

app/src/main/java/eu/kanade/tachiyomi/ui/main/MainActivity.kt

@@ -131,8 +131,6 @@ class MainActivity : BaseActivity() {
 
         super.onCreate(savedInstanceState)
 
-        val didMigration = Migrator.awaitAndRelease()
-
         // Do not let the launcher create a new activity http://stackoverflow.com/questions/16283079
         if (!isTaskRoot) {
             finish()
@@ -140,6 +138,11 @@ class MainActivity : BaseActivity() {
         }
 
         setComposeContent {
+            var didMigration by remember { mutableStateOf<Boolean?>(null) }
+            LaunchedEffect(Unit) {
+                didMigration = Migrator.awaitAndRelease()
+            }
+
             val context = LocalContext.current
 
             var incognito by remember { mutableStateOf(getIncognitoState.await(null)) }
@@ -242,7 +245,7 @@ class MainActivity : BaseActivity() {
                 ShowOnboarding()
             }
 
-            var showChangelog by remember { mutableStateOf(didMigration && !BuildConfig.DEBUG) }
+            var showChangelog by remember { mutableStateOf(didMigration == true && !BuildConfig.DEBUG) }
             if (showChangelog) {
                 AlertDialog(
                     onDismissRequest = { showChangelog = false },

app/src/main/java/eu/kanade/tachiyomi/ui/reader/ReaderViewModel.kt

@@ -145,18 +145,25 @@ class ReaderViewModel @JvmOverloads constructor(
 
     private var chapterToDownload: Download? = null
 
-    private val unfilteredChapterList by lazy {
+    private var unfilteredChapterListCache: List<tachiyomi.domain.chapter.model.Chapter>? = null
+    private suspend fun getUnfilteredChapterList(): List<tachiyomi.domain.chapter.model.Chapter> {
+        if (unfilteredChapterListCache == null) {
             val manga = manga!!
-        runBlocking { getChaptersByMangaId.await(manga.id, applyScanlatorFilter = false) }
+            unfilteredChapterListCache = getChaptersByMangaId.await(manga.id, applyScanlatorFilter = false)
+        }
+        return unfilteredChapterListCache!!
     }
 
     /**
      * Chapter list for the active manga. It's retrieved lazily and should be accessed for the first
      * time in a background thread to avoid blocking the UI.
      */
-    private val chapterList by lazy {
+    private var chapterListCache: List<ReaderChapter>? = null
+    private suspend fun getChapterList(): List<ReaderChapter> {
+        chapterListCache?.let { return it }
+
         val manga = manga!!
-        val chapters = runBlocking { getChaptersByMangaId.await(manga.id, applyScanlatorFilter = true) }
+        val chapters = getChaptersByMangaId.await(manga.id, applyScanlatorFilter = true)
 
         val selectedChapter = chapters.find { it.id == chapterId }
             ?: error("Requested chapter of id $chapterId not found in chapter list")
@@ -205,7 +212,7 @@ class ReaderViewModel @JvmOverloads constructor(
             else -> chapters
         }
 
-        chaptersForReader
+        val result = chaptersForReader
             .sortedWith(getChapterSort(manga, sortDescending = false))
             .run {
                 if (readerPreferences.skipDupe().get()) {
@@ -223,6 +230,8 @@ class ReaderViewModel @JvmOverloads constructor(
             }
             .map { it.toDbChapter() }
             .map(::ReaderChapter)
+        chapterListCache = result
+        return result
     }
 
     private val incognitoMode: Boolean by lazy { getIncognitoState.await(manga?.source) }
@@ -288,7 +297,7 @@ class ReaderViewModel @JvmOverloads constructor(
                     val source = sourceManager.getOrStub(manga.source)
                     loader = ChapterLoader(context, downloadManager, downloadProvider, manga, source)
 
-                    loadChapter(loader!!, chapterList.first { chapterId == it.chapter.id })
+                    loadChapter(loader!!, getChapterList().first { chapterId == it.chapter.id })
                     Result.success(true)
                 } else {
                     // Unlikely but okay
@@ -313,6 +322,7 @@ class ReaderViewModel @JvmOverloads constructor(
     ): ViewerChapters {
         loader.loadChapter(chapter)
 
+        val chapterList = getChapterList()
         val chapterPos = chapterList.indexOf(chapter)
         val newChapters = ViewerChapters(
             chapter,
@@ -511,11 +521,12 @@ class ReaderViewModel @JvmOverloads constructor(
      * If both conditions are satisfied enqueues chapter for delete
      * @param currentChapter current chapter, which is going to be marked as read.
      */
-    private fun deleteChapterIfNeeded(currentChapter: ReaderChapter) {
+    private suspend fun deleteChapterIfNeeded(currentChapter: ReaderChapter) {
         val removeAfterReadSlots = downloadPreferences.removeAfterReadSlots().get()
         if (removeAfterReadSlots == -1) return
 
         // Determine which chapter should be deleted and enqueue
+        val chapterList = getChapterList()
         val currentChapterPosition = chapterList.indexOf(currentChapter)
         val chapterToDelete = chapterList.getOrNull(currentChapterPosition - removeAfterReadSlots)
 
@@ -566,7 +577,7 @@ class ReaderViewModel @JvmOverloads constructor(
             .contains(LibraryPreferences.MARK_DUPLICATE_CHAPTER_READ_EXISTING)
         if (!markDuplicateAsRead) return
 
-        val duplicateUnreadChapters = unfilteredChapterList
+        val duplicateUnreadChapters = getUnfilteredChapterList()
             .mapNotNull { chapter ->
                 if (
                     !chapter.read &&
@@ -679,7 +690,7 @@ class ReaderViewModel @JvmOverloads constructor(
      */
     fun setMangaReadingMode(readingMode: ReadingMode) {
         val manga = manga ?: return
-        runBlocking(Dispatchers.IO) {
+        viewModelScope.launchIO {
             setMangaViewerFlags.awaitSetReadingMode(manga.id, readingMode.flagValue.toLong())
             val currChapters = state.value.viewerChapters
             if (currChapters != null) {

app/src/main/java/eu/kanade/tachiyomi/ui/updates/UpdatesScreenModel.kt

@@ -239,7 +239,7 @@ class UpdatesScreenModel(
         }
     }
 
-    private fun startDownloadingNow(chapterId: Long) {
+    private suspend fun startDownloadingNow(chapterId: Long) {
         downloadManager.startDownloadNow(chapterId)
     }
 

app/src/main/java/mihon/core/migration/Migrator.kt

@@ -35,7 +35,7 @@ object Migrator {
         result = null
     }
 
-    fun awaitAndRelease(): Boolean = runBlocking {
+    suspend fun awaitAndRelease(): Boolean {
-        await().also { release() }
+        return await().also { release() }
     }
 }

data/src/main/java/tachiyomi/data/TransactionContext.kt

@@ -5,6 +5,8 @@ import kotlinx.coroutines.Job
 import kotlinx.coroutines.asContextElement
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.suspendCancellableCoroutine
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.withContext
 import java.util.concurrent.RejectedExecutionException
 import kotlin.concurrent.atomics.AtomicInt
@@ -17,6 +19,10 @@ import kotlin.coroutines.EmptyCoroutineContext
 import kotlin.coroutines.coroutineContext
 import kotlin.coroutines.resume
 
+// Global mutex to serialize transaction entry and prevent thread pool exhaustion.
+// If you have multiple distinct database files/handlers, this should be a property of AndroidDatabaseHandler.
+private val transactionMutex = Mutex()
+
 /**
  * Returns the transaction dispatcher if we are on a transaction, or the database dispatchers.
  */
@@ -39,12 +45,32 @@ internal suspend fun AndroidDatabaseHandler.getCurrentDatabaseContext(): Corouti
  * The dispatcher used to execute the given [block] will utilize threads from SQLDelight's query executor.
  */
 internal suspend fun <T> AndroidDatabaseHandler.withTransaction(block: suspend () -> T): T {
-    // Use inherited transaction context if available, this allows nested suspending transactions.
+    val transactionElement = coroutineContext[TransactionElement]
-    val transactionContext =
+
-        coroutineContext[TransactionElement]?.transactionDispatcher ?: createTransactionContext()
+    // If we are already in a transaction, we don't need to lock the Mutex.
-    return withContext(transactionContext) {
+    // We just reuse the existing thread/context.
-        val transactionElement = coroutineContext[TransactionElement]!!
+    if (transactionElement != null) {
+        return withContext(transactionElement.transactionDispatcher) {
             transactionElement.acquire()
+            try {
+                db.transactionWithResult {
+                    runBlocking(transactionElement.transactionDispatcher) {
+                        block()
+                    }
+                }
+            } finally {
+                transactionElement.release()
+            }
+        }
+    }
+
+    // transaction: Acquire Mutex BEFORE acquiring a thread.
+    // This ensures we only block a real thread when we have exclusive access.
+    return transactionMutex.withLock {
+        val transactionContext = createTransactionContext()
+        withContext(transactionContext) {
+            val element = coroutineContext[TransactionElement]!!
+            element.acquire()
             try {
                 db.transactionWithResult {
                     runBlocking(transactionContext) {
@@ -52,7 +78,8 @@ internal suspend fun <T> AndroidDatabaseHandler.withTransaction(block: suspend (
                     }
                 }
             } finally {
-            transactionElement.release()
+                element.release()
+            }
         }
     }
 }

i18n/src/commonMain/moko-resources/base/strings.xml

@@ -171,6 +171,7 @@
 
     <!-- Operations -->
     <string name="loading">Loading…</string>
+    <string name="calculating">Calculating…</string>
     <string name="internal_error">InternalError: Check crash logs for further information</string>
 
     <!-- Shortcuts-->