Merge pull request #4105 from kiwix/Fixes#4104

Fixed: Sometimes application crashes due to a native crash when we frequently load the ZIM files in the reader.
This commit is contained in:
Kelson 2024-11-22 17:06:28 +01:00 committed by GitHub
commit fff3c3ec22
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 80 additions and 16 deletions

View File

@ -18,6 +18,7 @@
package org.kiwix.kiwixmobile.note package org.kiwix.kiwixmobile.note
import android.os.Build
import androidx.core.content.ContextCompat import androidx.core.content.ContextCompat
import androidx.core.content.edit import androidx.core.content.edit
import androidx.core.net.toUri import androidx.core.net.toUri
@ -179,8 +180,11 @@ class NoteFragmentTest : BaseActivityTest() {
assertNoteSaved() assertNoteSaved()
pressBack() pressBack()
} }
if (Build.VERSION.SDK_INT > Build.VERSION_CODES.N_MR1) {
// temporary disabled on Android 25
LeakAssertions.assertNoLeaks() LeakAssertions.assertNoLeaks()
} }
}
@Test @Test
fun testZimFileOpenedAfterOpeningNoteOnNotesScreen() { fun testZimFileOpenedAfterOpeningNoteOnNotesScreen() {

View File

@ -18,6 +18,7 @@
package org.kiwix.kiwixmobile.page.history package org.kiwix.kiwixmobile.page.history
import android.os.Build
import androidx.core.content.ContextCompat import androidx.core.content.ContextCompat
import androidx.core.content.edit import androidx.core.content.edit
import androidx.core.net.toUri import androidx.core.net.toUri
@ -151,8 +152,11 @@ class NavigationHistoryTest : BaseActivityTest() {
pressBack() pressBack()
pressBack() pressBack()
} }
if (Build.VERSION.SDK_INT > Build.VERSION_CODES.N_MR1) {
// temporary disabled on Android 25
LeakAssertions.assertNoLeaks() LeakAssertions.assertNoLeaks()
} }
}
@After @After
fun finish() { fun finish() {

View File

@ -18,6 +18,7 @@
package org.kiwix.kiwixmobile.reader package org.kiwix.kiwixmobile.reader
import android.os.Build
import androidx.core.content.ContextCompat import androidx.core.content.ContextCompat
import androidx.core.content.edit import androidx.core.content.edit
import androidx.core.net.toUri import androidx.core.net.toUri
@ -137,8 +138,11 @@ class KiwixReaderFragmentTest : BaseActivityTest() {
pressBack() pressBack()
checkZimFileLoadedSuccessful(R.id.readerFragment) checkZimFileLoadedSuccessful(R.id.readerFragment)
} }
if (Build.VERSION.SDK_INT > Build.VERSION_CODES.N_MR1) {
// temporary disabled on Android 25
LeakAssertions.assertNoLeaks() LeakAssertions.assertNoLeaks()
} }
}
@Test @Test
fun testZimFileRendering() { fun testZimFileRendering() {

View File

@ -17,6 +17,7 @@
*/ */
package org.kiwix.kiwixmobile.search package org.kiwix.kiwixmobile.search
import android.os.Build
import androidx.core.content.ContextCompat import androidx.core.content.ContextCompat
import androidx.core.content.edit import androidx.core.content.edit
import androidx.core.net.toUri import androidx.core.net.toUri
@ -221,8 +222,11 @@ class SearchFragmentTest : BaseActivityTest() {
assertArticleLoaded() assertArticleLoaded()
} }
removeTemporaryZimFilesToFreeUpDeviceStorage() removeTemporaryZimFilesToFreeUpDeviceStorage()
if (Build.VERSION.SDK_INT > Build.VERSION_CODES.N_MR1) {
// temporary disabled on Android 25
LeakAssertions.assertNoLeaks() LeakAssertions.assertNoLeaks()
} }
}
@Test @Test
fun testConcurrencyOfSearch() = runBlocking { fun testConcurrencyOfSearch() = runBlocking {

View File

@ -57,6 +57,7 @@ import org.kiwix.kiwixmobile.zimManager.Fat32Checker
import org.kiwix.kiwixmobile.zimManager.Fat32Checker.Companion.FOUR_GIGABYTES_IN_KILOBYTES import org.kiwix.kiwixmobile.zimManager.Fat32Checker.Companion.FOUR_GIGABYTES_IN_KILOBYTES
import org.kiwix.kiwixmobile.zimManager.Fat32Checker.FileSystemState.CannotWrite4GbFile import org.kiwix.kiwixmobile.zimManager.Fat32Checker.FileSystemState.CannotWrite4GbFile
import org.kiwix.kiwixmobile.zimManager.Fat32Checker.FileSystemState.DetectingFileSystem import org.kiwix.kiwixmobile.zimManager.Fat32Checker.FileSystemState.DetectingFileSystem
import org.kiwix.libzim.Archive
import java.io.File import java.io.File
import java.io.FileInputStream import java.io.FileInputStream
import java.io.FileNotFoundException import java.io.FileNotFoundException
@ -418,13 +419,16 @@ class CopyMoveFileHandler @Inject constructor(
} }
suspend fun isValidZimFile(destinationFile: File): Boolean { suspend fun isValidZimFile(destinationFile: File): Boolean {
var archive: Archive? = null
return try { return try {
// create archive object, and check if it has the mainEntry or not to validate the ZIM file. // create archive object, and check if it has the mainEntry or not to validate the ZIM file.
val archive = ZimReaderSource(destinationFile).createArchive() archive = ZimReaderSource(destinationFile).createArchive()
archive?.hasMainEntry() == true archive?.hasMainEntry() == true
} catch (ignore: Exception) { } catch (ignore: Exception) {
// if it is a invalid ZIM file // if it is a invalid ZIM file
false false
} finally {
archive?.dispose()
} }
} }

View File

@ -31,7 +31,6 @@ import android.view.View.VISIBLE
import android.view.ViewGroup import android.view.ViewGroup
import androidx.appcompat.app.AppCompatActivity import androidx.appcompat.app.AppCompatActivity
import androidx.drawerlayout.widget.DrawerLayout import androidx.drawerlayout.widget.DrawerLayout
import androidx.lifecycle.lifecycleScope
import com.google.android.material.bottomnavigation.BottomNavigationView import com.google.android.material.bottomnavigation.BottomNavigationView
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import org.kiwix.kiwixmobile.R import org.kiwix.kiwixmobile.R
@ -93,7 +92,7 @@ class KiwixReaderFragment : CoreReaderFragment() {
private fun openPageInBookFromNavigationArguments() { private fun openPageInBookFromNavigationArguments() {
showProgressBarWithProgress(30) showProgressBarWithProgress(30)
val args = KiwixReaderFragmentArgs.fromBundle(requireArguments()) val args = KiwixReaderFragmentArgs.fromBundle(requireArguments())
lifecycleScope.launch { coreReaderLifeCycleScope?.launch {
if (args.pageUrl.isNotEmpty()) { if (args.pageUrl.isNotEmpty()) {
if (args.zimFileUri.isNotEmpty()) { if (args.zimFileUri.isNotEmpty()) {
tryOpeningZimFile(args.zimFileUri) tryOpeningZimFile(args.zimFileUri)
@ -120,6 +119,12 @@ class KiwixReaderFragment : CoreReaderFragment() {
} }
private suspend fun tryOpeningZimFile(zimFileUri: String) { private suspend fun tryOpeningZimFile(zimFileUri: String) {
// Stop any ongoing WebView loading and clear the WebView list
// before setting a new ZIM file to the reader. This helps prevent native crashes.
// The WebView's `shouldInterceptRequest` method continues to be invoked until the WebView is
// fully destroyed, which can cause a native crash. This happens because a new ZIM file is set
// in the reader while the WebView is still trying to access content from the old archive.
stopOngoingLoadingAndClearWebViewList()
// Close the previously opened book in the reader before opening a new ZIM file // Close the previously opened book in the reader before opening a new ZIM file
// to avoid native crashes due to "null pointer dereference." These crashes can occur // to avoid native crashes due to "null pointer dereference." These crashes can occur
// when setting a new ZIM file in the archive while the previous one is being disposed of. // when setting a new ZIM file in the archive while the previous one is being disposed of.
@ -143,7 +148,6 @@ class KiwixReaderFragment : CoreReaderFragment() {
return return
} }
val zimReaderSource = ZimReaderSource(File(filePath)) val zimReaderSource = ZimReaderSource(File(filePath))
clearWebViewListIfNotPreviouslyOpenZimFile(zimReaderSource)
openZimFile(zimReaderSource) openZimFile(zimReaderSource)
} }
@ -252,7 +256,7 @@ class KiwixReaderFragment : CoreReaderFragment() {
) { ) {
when (restoreOrigin) { when (restoreOrigin) {
FromExternalLaunch -> { FromExternalLaunch -> {
lifecycleScope.launch { coreReaderLifeCycleScope?.launch {
val settings = val settings =
requireActivity().getSharedPreferences(SharedPreferenceUtil.PREF_KIWIX_MOBILE, 0) requireActivity().getSharedPreferences(SharedPreferenceUtil.PREF_KIWIX_MOBILE, 0)
val zimReaderSource = fromDatabaseValue(settings.getString(TAG_CURRENT_FILE, null)) val zimReaderSource = fromDatabaseValue(settings.getString(TAG_CURRENT_FILE, null))

View File

@ -94,6 +94,10 @@ import io.reactivex.Flowable
import io.reactivex.android.schedulers.AndroidSchedulers import io.reactivex.android.schedulers.AndroidSchedulers
import io.reactivex.disposables.Disposable import io.reactivex.disposables.Disposable
import io.reactivex.processors.BehaviorProcessor import io.reactivex.processors.BehaviorProcessor
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import org.json.JSONArray import org.json.JSONArray
import org.json.JSONException import org.json.JSONException
@ -323,6 +327,15 @@ abstract class CoreReaderFragment :
private var navigationHistoryList: MutableList<NavigationHistoryListItem> = ArrayList() private var navigationHistoryList: MutableList<NavigationHistoryListItem> = ArrayList()
private var isReadSelection = false private var isReadSelection = false
private var isReadAloudServiceRunning = false private var isReadAloudServiceRunning = false
private var readerLifeCycleScope: CoroutineScope? = null
val coreReaderLifeCycleScope: CoroutineScope?
get() {
if (readerLifeCycleScope == null) {
readerLifeCycleScope = CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate)
}
return readerLifeCycleScope
}
private var storagePermissionForNotesLauncher: ActivityResultLauncher<String>? = private var storagePermissionForNotesLauncher: ActivityResultLauncher<String>? =
registerForActivityResult( registerForActivityResult(
@ -1198,6 +1211,12 @@ abstract class CoreReaderFragment :
override fun onDestroyView() { override fun onDestroyView() {
super.onDestroyView() super.onDestroyView()
try {
readerLifeCycleScope?.cancel()
readerLifeCycleScope = null
} catch (ignore: Exception) {
ignore.printStackTrace()
}
if (sharedPreferenceUtil?.showIntro() == true) { if (sharedPreferenceUtil?.showIntro() == true) {
val activity = requireActivity() as AppCompatActivity? val activity = requireActivity() as AppCompatActivity?
activity?.setSupportActionBar(null) activity?.setSupportActionBar(null)
@ -1208,14 +1227,13 @@ abstract class CoreReaderFragment :
tabCallback = null tabCallback = null
hideBackToTopTimer?.cancel() hideBackToTopTimer?.cancel()
hideBackToTopTimer = null hideBackToTopTimer = null
webViewList.clear() stopOngoingLoadingAndClearWebViewList()
actionBar = null actionBar = null
mainMenu = null mainMenu = null
tabRecyclerView?.adapter = null tabRecyclerView?.adapter = null
tableDrawerRight?.adapter = null tableDrawerRight?.adapter = null
tableDrawerAdapter = null tableDrawerAdapter = null
tabsAdapter = null tabsAdapter = null
webViewList.clear()
tempWebViewListForUndo.clear() tempWebViewListForUndo.clear()
// create a base Activity class that class this. // create a base Activity class that class this.
deleteCachedFiles(requireActivity()) deleteCachedFiles(requireActivity())
@ -1714,13 +1732,36 @@ abstract class CoreReaderFragment :
} }
} }
fun clearWebViewListIfNotPreviouslyOpenZimFile(zimReaderSource: ZimReaderSource) { private fun clearWebViewListIfNotPreviouslyOpenZimFile(zimReaderSource: ZimReaderSource?) {
try {
if (isNotPreviouslyOpenZim(zimReaderSource)) { if (isNotPreviouslyOpenZim(zimReaderSource)) {
webViewList.clear() stopOngoingLoadingAndClearWebViewList()
}
}
protected fun stopOngoingLoadingAndClearWebViewList() {
try {
webViewList.apply {
forEach { webView ->
// Stop any ongoing loading of the WebView
webView.stopLoading()
// Clear the navigation history of the WebView
webView.clearHistory()
// Clear cached resources to prevent loading old content
webView.clearCache(true)
// Pause any ongoing activity in the WebView to prevent resource usage
webView.onPause()
// Forcefully destroy the WebView before setting the new ZIM file
// to ensure that it does not continue attempting to load internal links
// from the previous ZIM file, which could cause errors.
webView.destroy()
}
// Clear the WebView list after destroying the WebViews
clear()
} }
} catch (e: IOException) { } catch (e: IOException) {
e.printStackTrace() e.printStackTrace()
// Clear the WebView list in case of an error
webViewList.clear()
} }
} }

View File

@ -30,7 +30,6 @@ import androidx.appcompat.app.AppCompatActivity
import androidx.appcompat.widget.Toolbar import androidx.appcompat.widget.Toolbar
import androidx.core.net.toUri import androidx.core.net.toUri
import androidx.drawerlayout.widget.DrawerLayout import androidx.drawerlayout.widget.DrawerLayout
import androidx.lifecycle.lifecycleScope
import androidx.navigation.fragment.findNavController import androidx.navigation.fragment.findNavController
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import org.kiwix.kiwixmobile.core.R.dimen import org.kiwix.kiwixmobile.core.R.dimen
@ -188,7 +187,7 @@ class CustomReaderFragment : CoreReaderFragment() {
private fun openObbOrZim() { private fun openObbOrZim() {
customFileValidator.validate( customFileValidator.validate(
onFilesFound = { onFilesFound = {
lifecycleScope.launch { coreReaderLifeCycleScope?.launch {
when (it) { when (it) {
is ValidationState.HasFile -> { is ValidationState.HasFile -> {
openZimFile( openZimFile(