From 82b40096d22d38cc63d1b5d6fb9ec63737466752 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Mon, 17 Nov 2025 22:33:58 +0530 Subject: [PATCH] Fixed: `Input dispatching timed out` error on `Manager.readFile`. * Moved the initialization on `LibkiwixBookOnDisk` on IO thread instead of main thread. Now, the manager will read the file on IO thread before any method runs that depends on it. It will run only once. * Improved the `localBooksFlow`. * Refactored `LibkiwixBookOnDisk` similarly, since it also used the same initialization approach. * Since we now use `tempLibrary` for parsing the online library content, we no longer need the library and manager objects provided via Dagger. * Fixed: The bookmark icon was not showing when adding a bookmark for a new book, deleting it, and then adding it again. --- .../di/components/KiwixComponent.kt | 4 +- .../kiwix/kiwixmobile/di/modules/JNIModule.kt | 52 ------------- .../zimManager/OnlineLibraryManager.kt | 9 +-- .../core/dao/LibkiwixBookOnDisk.kt | 42 +++++++--- .../kiwixmobile/core/dao/LibkiwixBookmarks.kt | 76 ++++++++++++------- 5 files changed, 84 insertions(+), 99 deletions(-) delete mode 100644 app/src/main/java/org/kiwix/kiwixmobile/di/modules/JNIModule.kt diff --git a/app/src/main/java/org/kiwix/kiwixmobile/di/components/KiwixComponent.kt b/app/src/main/java/org/kiwix/kiwixmobile/di/components/KiwixComponent.kt index 377eb5c9bc..6b91a107ae 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/di/components/KiwixComponent.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/di/components/KiwixComponent.kt @@ -21,13 +21,12 @@ package org.kiwix.kiwixmobile.di.components import dagger.Component import org.kiwix.kiwixmobile.core.data.ObjectBoxDataMigrationHandler import org.kiwix.kiwixmobile.core.di.components.CoreComponent -import org.kiwix.kiwixmobile.migration.di.module.MigrationModule import org.kiwix.kiwixmobile.di.KiwixScope import org.kiwix.kiwixmobile.di.components.ServiceComponent.Builder -import org.kiwix.kiwixmobile.di.modules.JNIModule import org.kiwix.kiwixmobile.di.modules.KiwixModule import org.kiwix.kiwixmobile.di.modules.KiwixViewModelModule import org.kiwix.kiwixmobile.migration.di.module.DatabaseModule +import org.kiwix.kiwixmobile.migration.di.module.MigrationModule import org.kiwix.kiwixmobile.storage.StorageSelectDialog import org.kiwix.kiwixmobile.zimManager.OnlineLibraryManager @@ -37,7 +36,6 @@ import org.kiwix.kiwixmobile.zimManager.OnlineLibraryManager modules = [ KiwixViewModelModule::class, KiwixModule::class, - JNIModule::class, MigrationModule::class, DatabaseModule::class ] diff --git a/app/src/main/java/org/kiwix/kiwixmobile/di/modules/JNIModule.kt b/app/src/main/java/org/kiwix/kiwixmobile/di/modules/JNIModule.kt deleted file mode 100644 index 6785167cdb..0000000000 --- a/app/src/main/java/org/kiwix/kiwixmobile/di/modules/JNIModule.kt +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Kiwix Android - * Copyright (c) 2025 Kiwix - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - * - */ - -package org.kiwix.kiwixmobile.di.modules - -import dagger.Module -import dagger.Provides -import org.kiwix.kiwixmobile.di.KiwixScope -import org.kiwix.kiwixmobile.zimManager.OnlineLibraryManager -import org.kiwix.libkiwix.Library -import org.kiwix.libkiwix.Manager -import javax.inject.Named - -@Module -class JNIModule { - @Provides - @Named(ONLINE_BOOKS_LIBRARY) - @KiwixScope - fun provideOnlineBooksLibrary(): Library = Library() - - @Provides - @Named(ONLINE_BOOKS_MANAGER) - @KiwixScope - fun providesOnlineBooksManager( - @Named(ONLINE_BOOKS_LIBRARY) library: Library - ): Manager = Manager(library) - - @Provides - @KiwixScope - fun provideOnlineLibraryManager( - @Named(ONLINE_BOOKS_LIBRARY) library: Library, - @Named(ONLINE_BOOKS_MANAGER) manager: Manager, - ) = OnlineLibraryManager(library, manager) -} - -const val ONLINE_BOOKS_LIBRARY = "onlineBookLibrary" -const val ONLINE_BOOKS_MANAGER = "onlineBookManager" diff --git a/app/src/main/java/org/kiwix/kiwixmobile/zimManager/OnlineLibraryManager.kt b/app/src/main/java/org/kiwix/kiwixmobile/zimManager/OnlineLibraryManager.kt index 7c57912af3..2491857815 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/zimManager/OnlineLibraryManager.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/zimManager/OnlineLibraryManager.kt @@ -24,21 +24,14 @@ import kotlinx.coroutines.withContext import org.kiwix.kiwixmobile.core.data.remote.KiwixService.Companion.OPDS_LIBRARY_ENDPOINT import org.kiwix.kiwixmobile.core.downloader.downloadManager.ZERO import org.kiwix.kiwixmobile.core.entity.LibkiwixBook -import org.kiwix.kiwixmobile.di.modules.ONLINE_BOOKS_LIBRARY -import org.kiwix.kiwixmobile.di.modules.ONLINE_BOOKS_MANAGER import org.kiwix.libkiwix.Library import org.kiwix.libkiwix.Manager import org.xmlpull.v1.XmlPullParser import org.xmlpull.v1.XmlPullParserFactory import java.io.StringReader import javax.inject.Inject -import javax.inject.Named -@Suppress("UnusedPrivateProperty") -class OnlineLibraryManager @Inject constructor( - @Named(ONLINE_BOOKS_LIBRARY) private val library: Library, - @Named(ONLINE_BOOKS_MANAGER) private val manager: Manager, -) { +class OnlineLibraryManager @Inject constructor() { var totalResult = 0 suspend fun parseOPDSStreamAndGetBooks( content: String?, diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookOnDisk.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookOnDisk.kt index 4f88a32a6c..726b1e3ced 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookOnDisk.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookOnDisk.kt @@ -24,11 +24,13 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.mapLatest import kotlinx.coroutines.launch -import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import org.kiwix.kiwixmobile.core.dao.LibkiwixBookmarks.Companion.TAG import org.kiwix.kiwixmobile.core.di.modules.LOCAL_BOOKS_LIBRARY @@ -50,6 +52,8 @@ class LibkiwixBookOnDisk @Inject constructor( @Named(LOCAL_BOOKS_MANAGER) private val manager: Manager, private val sharedPreferenceUtil: SharedPreferenceUtil ) { + private val initMutex = Mutex() + private var isManagerInitialized = false private var libraryBooksList: List = arrayListOf() private var localBooksList: List = arrayListOf() @@ -73,18 +77,29 @@ class LibkiwixBookOnDisk @Inject constructor( File("$localBookFolderPath/library.xml") } - init { - // Check if ZIM files folder exist if not then create the folder first. - if (runBlocking { !File(localBookFolderPath).isFileExist() }) File(localBookFolderPath).mkdir() - // Check if library file exist if not then create the file to save the library with book information. - if (runBlocking { !libraryFile.isFileExist() }) libraryFile.createNewFile() - // set up manager to read the library from this file - manager.readFile(libraryFile.canonicalPath) + private suspend fun ensureInitialized(dispatcher: CoroutineDispatcher = Dispatchers.IO) { + if (isManagerInitialized) return + initMutex.withLock { + if (isManagerInitialized) return@withLock true + withContext(dispatcher) { + // Check if ZIM files folder exist if not then create the folder first. + if (!File(localBookFolderPath).isFileExist()) { + File(localBookFolderPath).mkdirs() + } + // Check if library file exist if not then create the file to save the library with book information. + if (!libraryFile.isFileExist()) { + libraryFile.createNewFile() + } + // set up manager to read the library from this file + manager.readFile(libraryFile.canonicalPath) + isManagerInitialized = true + } + } } @Suppress("InjectDispatcher") - private val localBooksFlow: MutableStateFlow> by lazy { - MutableStateFlow>(emptyList()).also { flow -> + private val localBooksFlow: MutableStateFlow?> by lazy { + MutableStateFlow?>(null).also { flow -> CoroutineScope(Dispatchers.IO).launch { runCatching { flow.emit(getBooksList()) @@ -95,6 +110,8 @@ class LibkiwixBookOnDisk @Inject constructor( private suspend fun getBooksList(dispatcher: CoroutineDispatcher = Dispatchers.IO): List = withContext(dispatcher) { + // if reading library failed, return empty list + ensureInitialized() if (!booksChanged && localBooksList.isNotEmpty()) { // No changes, return the cached data return@withContext localBooksList.distinctBy(LibkiwixBook::path) @@ -118,6 +135,7 @@ class LibkiwixBookOnDisk @Inject constructor( @OptIn(ExperimentalCoroutinesApi::class) fun books(dispatcher: CoroutineDispatcher = Dispatchers.IO) = localBooksFlow + .filterNotNull() .mapLatest { booksList -> removeBooksThatAreInTrashFolder(booksList) removeBooksThatDoNotExist(booksList.toMutableList()) @@ -143,6 +161,7 @@ class LibkiwixBookOnDisk @Inject constructor( @Suppress("InjectDispatcher") suspend fun insert(libkiwixBooks: List) { withContext(Dispatchers.IO) { + ensureInitialized() val existingBookIds = library.booksIds.toSet() val existingBookPaths = existingBookIds .mapNotNull { id -> library.getBookById(id)?.path } @@ -207,6 +226,7 @@ class LibkiwixBookOnDisk @Inject constructor( suspend fun delete(books: List) { runCatching { + ensureInitialized() books.forEach { library.removeBookById(it.id) } @@ -217,6 +237,7 @@ class LibkiwixBookOnDisk @Inject constructor( suspend fun delete(bookId: String) { runCatching { + ensureInitialized() library.removeBookById(bookId) writeBookMarksAndSaveLibraryToFile() updateLocalBooksFlow() @@ -236,6 +257,7 @@ class LibkiwixBookOnDisk @Inject constructor( * to prevent potential data loss and ensures that the library holds the updated ZIM file data. */ private suspend fun writeBookMarksAndSaveLibraryToFile() { + ensureInitialized() // Save the library, which contains ZIM file data. library.writeToFile(libraryFile.canonicalPath) // set the bookmark change to true so that libkiwix will return the new data. diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt index 18b96419af..e06f026fd3 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt @@ -28,7 +28,8 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch -import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import org.kiwix.kiwixmobile.core.CoreApp import org.kiwix.kiwixmobile.core.R @@ -70,6 +71,8 @@ class LibkiwixBookmarks @Inject constructor( private var bookmarksChanged: Boolean = false private var bookmarkList: List = arrayListOf() private var libraryBooksList: List = arrayListOf() + private val initMutex = Mutex() + private var initialized: Boolean = false @Suppress("InjectDispatcher") private val bookmarkListFlow: MutableStateFlow> by lazy { @@ -101,17 +104,29 @@ class LibkiwixBookmarks @Inject constructor( File("$bookmarksFolderPath/library.xml") } - init { - // Check if bookmark folder exist if not then create the folder first. - if (runBlocking { !File(bookmarksFolderPath).isFileExist() }) File(bookmarksFolderPath).mkdir() - // Check if library file exist if not then create the file to save the library with book information. - if (runBlocking { !libraryFile.isFileExist() }) libraryFile.createNewFile() - // set up manager to read the library from this file - manager.readFile(libraryFile.canonicalPath) - // Check if bookmark file exist if not then create the file to save the bookmarks. - if (runBlocking { !bookmarkFile.isFileExist() }) bookmarkFile.createNewFile() - // set up manager to read the bookmarks from this file - manager.readBookmarkFile(bookmarkFile.canonicalPath) + /** + * Ensure initialization runs once. This method performs all file I/O and manager setup + * on Dispatchers.IO so it won't block the main thread. It is safe to call multiple times. + */ + private suspend fun ensureInitialized(dispatcher: CoroutineDispatcher = Dispatchers.IO) { + if (initialized) return + + initMutex.withLock { + if (initialized) return + withContext(dispatcher) { + // Check if bookmark folder exist if not then create the folder first. + if (!File(bookmarksFolderPath).isFileExist()) File(bookmarksFolderPath).mkdir() + // Check if library file exist if not then create the file to save the library with book information. + if (!libraryFile.isFileExist()) libraryFile.createNewFile() + // set up manager to read the library from this file + manager.readFile(libraryFile.canonicalPath) + // Check if bookmark file exist if not then create the file to save the bookmarks. + if (!bookmarkFile.isFileExist()) bookmarkFile.createNewFile() + // set up manager to read the bookmarks from this file + manager.readBookmarkFile(bookmarkFile.canonicalPath) + initialized = true + } + } } fun bookmarks(): Flow> = @@ -124,14 +139,16 @@ class LibkiwixBookmarks @Inject constructor( deleteBookmarks(pagesToDelete as List) @Suppress("InjectDispatcher") - suspend fun getCurrentZimBookmarksUrl(zimFileReader: ZimFileReader?): List = - withContext(Dispatchers.IO) { - return@withContext zimFileReader?.let { reader -> + suspend fun getCurrentZimBookmarksUrl(zimFileReader: ZimFileReader?): List { + ensureInitialized() + return withContext(Dispatchers.IO) { + zimFileReader?.let { reader -> getBookmarksList() .filter { it.zimId == reader.id } .map(LibkiwixBookmarkItem::bookmarkUrl) }.orEmpty() } + } @Suppress("InjectDispatcher") fun bookmarkUrlsForCurrentBook(zimId: String): Flow> = @@ -150,6 +167,7 @@ class LibkiwixBookmarks @Inject constructor( libkiwixBookmarkItem: LibkiwixBookmarkItem, shouldWriteBookmarkToFile: Boolean = true ) { + ensureInitialized() if (!isBookMarkExist(libkiwixBookmarkItem)) { addBookToLibraryIfNotExist(libkiwixBookmarkItem.libKiwixBook) val bookmark = @@ -178,7 +196,8 @@ class LibkiwixBookmarks @Inject constructor( } suspend fun addBookToLibrary(file: File? = null, archive: Archive? = null) { - try { + ensureInitialized() + runCatching { bookmarksChanged = true val book = Book().apply { @@ -190,10 +209,10 @@ class LibkiwixBookmarks @Inject constructor( } addBookToLibraryIfNotExist(book) updateFlowBookmarkList() - } catch (ignore: Exception) { + }.onFailure { Log.e( TAG, - "Error: Couldn't add the book to library.\nOriginal exception = $ignore" + "Error: Couldn't add the book to library.\nOriginal exception = $it" ) } } @@ -228,14 +247,13 @@ class LibkiwixBookmarks @Inject constructor( bookmarks: List, dispatcher: CoroutineDispatcher = Dispatchers.IO ) { - bookmarks.map { library.removeBookmark(it.zimId, it.bookmarkUrl) } - .also { - CoroutineScope(dispatcher).launch { - writeBookMarksAndSaveLibraryToFile() - updateFlowBookmarkList() - removeBookFromLibraryIfNoRelatedBookmarksAreStored(dispatcher, bookmarks) - } - } + CoroutineScope(dispatcher).launch { + ensureInitialized() + bookmarks.map { library.removeBookmark(it.zimId, it.bookmarkUrl) } + writeBookMarksAndSaveLibraryToFile() + updateFlowBookmarkList() + removeBookFromLibraryIfNoRelatedBookmarksAreStored(dispatcher, bookmarks) + } } fun deleteBookmark(bookId: String, bookmarkUrl: String) { @@ -255,6 +273,7 @@ class LibkiwixBookmarks @Inject constructor( dispatcher: CoroutineDispatcher, deletedBookmarks: List ) { + ensureInitialized() withContext(dispatcher) { val currentBookmarks = getBookmarksList() val deletedZimIds = deletedBookmarks.map { it.zimId }.distinct() @@ -263,6 +282,7 @@ class LibkiwixBookmarks @Inject constructor( val stillExists = currentBookmarks.any { it.zimId == zimId } if (!stillExists) { library.removeBookById(zimId) + libraryBooksList = library.booksIds.toList() Log.d(TAG, "Removed book from library since no bookmarks exist for: $zimId") } } @@ -275,6 +295,7 @@ class LibkiwixBookmarks @Inject constructor( * to prevent potential data loss and ensures that the library holds the updated ZIM file paths and favicons. */ private suspend fun writeBookMarksAndSaveLibraryToFile() { + ensureInitialized() // Save the library, which contains ZIM file paths and favicons, to a file. library.writeToFile(libraryFile.canonicalPath) @@ -286,6 +307,7 @@ class LibkiwixBookmarks @Inject constructor( @Suppress("ReturnCount") private suspend fun getBookmarksList(): List { + ensureInitialized() if (!bookmarksChanged && bookmarkList.isNotEmpty()) { // No changes, return the cached data return bookmarkList.distinctBy(LibkiwixBookmarkItem::bookmarkUrl) @@ -400,6 +422,7 @@ class LibkiwixBookmarks @Inject constructor( // Export the `bookmark.xml` file to the `Download/org.kiwix/` directory of internal storage. suspend fun exportBookmark() { + ensureInitialized() try { val bookmarkDestinationFile = exportedFile("bookmark.xml") bookmarkFile.inputStream().use { inputStream -> @@ -438,6 +461,7 @@ class LibkiwixBookmarks @Inject constructor( } suspend fun importBookmarks(bookmarkFile: File) { + ensureInitialized() // Create a temporary library manager to import the bookmarks. val tempLibrary = Library() Manager(tempLibrary).apply {