From 1198a8c4956f3ff67adee412c2ec541c1cd44b9e Mon Sep 17 00:00:00 2001 From: Tom Levy Date: Mon, 18 Mar 2024 14:59:31 +0000 Subject: [PATCH 1/3] Report OutOfMemoryError when importing and exporting messages When importing/exporting large files we can run out of memory, in which case we should display a toast to the user (as we do for other exceptions). Previously, the OutOfMemoryError was not caught because it inherits from Error, not from Exception. (We have to manually call .toString() because Context.showErrorToast() from Commons takes an Exception or a String, not Throwable.) --- .../org/fossify/messages/activities/SettingsActivity.kt | 4 ++-- .../kotlin/org/fossify/messages/helpers/MessagesImporter.kt | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/src/main/kotlin/org/fossify/messages/activities/SettingsActivity.kt b/app/src/main/kotlin/org/fossify/messages/activities/SettingsActivity.kt index a31f1eb0..454fd92d 100644 --- a/app/src/main/kotlin/org/fossify/messages/activities/SettingsActivity.kt +++ b/app/src/main/kotlin/org/fossify/messages/activities/SettingsActivity.kt @@ -134,8 +134,8 @@ class SettingsActivity : SimpleActivity() { } toast(org.fossify.commons.R.string.exporting_successful) } - } catch (e: Exception) { - showErrorToast(e) + } catch (e: Throwable) { // also catch OutOfMemoryError etc. + showErrorToast(e.toString()) } } } diff --git a/app/src/main/kotlin/org/fossify/messages/helpers/MessagesImporter.kt b/app/src/main/kotlin/org/fossify/messages/helpers/MessagesImporter.kt index 9faa06a6..6f78feb2 100644 --- a/app/src/main/kotlin/org/fossify/messages/helpers/MessagesImporter.kt +++ b/app/src/main/kotlin/org/fossify/messages/helpers/MessagesImporter.kt @@ -32,8 +32,8 @@ class MessagesImporter(private val activity: SimpleActivity) { } else { importJson(uri) } - } catch (e: Exception) { - activity.showErrorToast(e) + } catch (e: Throwable) { // also catch OutOfMemoryError etc. + activity.showErrorToast(e.toString()) } } From 9534a1031a9185556460f7ea68deb68bcca1464e Mon Sep 17 00:00:00 2001 From: Tom Levy Date: Mon, 18 Mar 2024 15:31:42 +0000 Subject: [PATCH 2/3] Delete exported file on error If ActivityResultContracts.CreateDocument launches the native Files app (com.android.documentsui), then it will create the file before the Uri is returned to us. So if we don't complete the export (due to an exception or because there are no messages) we should delete the file, otherwise there will be an empty/incomplete file left behind. --- .../fossify/messages/activities/SettingsActivity.kt | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/app/src/main/kotlin/org/fossify/messages/activities/SettingsActivity.kt b/app/src/main/kotlin/org/fossify/messages/activities/SettingsActivity.kt index 454fd92d..a66230a8 100644 --- a/app/src/main/kotlin/org/fossify/messages/activities/SettingsActivity.kt +++ b/app/src/main/kotlin/org/fossify/messages/activities/SettingsActivity.kt @@ -5,6 +5,7 @@ import android.content.Intent import android.net.Uri import android.os.Build import android.os.Bundle +import android.provider.DocumentsContract import androidx.activity.result.contract.ActivityResultContracts import kotlinx.serialization.encodeToString import kotlinx.serialization.json.Json @@ -119,6 +120,7 @@ class SettingsActivity : SimpleActivity() { private fun exportMessages(uri: Uri) { ensureBackgroundThread { + var success = false try { MessagesReader(this).getMessagesToExport(config.exportSms, config.exportMms) { messagesToExport -> if (messagesToExport.isEmpty()) { @@ -132,10 +134,20 @@ class SettingsActivity : SimpleActivity() { outputStream.use { it.write(jsonString.toByteArray()) } + success = true toast(org.fossify.commons.R.string.exporting_successful) } } catch (e: Throwable) { // also catch OutOfMemoryError etc. showErrorToast(e.toString()) + } finally { + if (!success) { + // delete the file to avoid leaving behind an empty/corrupt file + try { + DocumentsContract.deleteDocument(contentResolver, uri) + } catch (ignored: Exception) { + // ignored because we don't want to overwhelm the user with two error messages + } + } } } } From a7edeae6f33d82016f35868f7933606485e1bcd7 Mon Sep 17 00:00:00 2001 From: Tom Levy Date: Mon, 18 Mar 2024 17:28:41 +0000 Subject: [PATCH 3/3] Use streams to encode and decode the JSON backups This significantly reduces memory usage. (We buffer the OutputStream because kotlinx.serialization flushes its internal buffers excessively[1]. We also buffer the InputStream for consistency, even though kotlinx.serialization uses adequate buffering; there is no performance impact because BufferedInputStream cascades harmlessly[2].) The functions encodeToStream() and decodeFromStream() are marked as experimental, but this is a pretty fundamental use case so surely it will continue to be supported in the future (maybe with minor changes). Fixes #6. [1] https://github.com/Kotlin/kotlinx.serialization/blob/v1.6.3/formats/json/jvmMain/src/kotlinx/serialization/json/internal/JvmJsonStreams.kt#L46 [2] https://github.com/openjdk/jdk/blob/jdk-23%2B14/src/java.base/share/classes/java/io/BufferedInputStream.java#L339 --- .../fossify/messages/activities/SettingsActivity.kt | 10 ++++------ .../org/fossify/messages/helpers/MessagesImporter.kt | 8 ++++---- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/app/src/main/kotlin/org/fossify/messages/activities/SettingsActivity.kt b/app/src/main/kotlin/org/fossify/messages/activities/SettingsActivity.kt index a66230a8..b1cf65e6 100644 --- a/app/src/main/kotlin/org/fossify/messages/activities/SettingsActivity.kt +++ b/app/src/main/kotlin/org/fossify/messages/activities/SettingsActivity.kt @@ -7,8 +7,8 @@ import android.os.Build import android.os.Bundle import android.provider.DocumentsContract import androidx.activity.result.contract.ActivityResultContracts -import kotlinx.serialization.encodeToString import kotlinx.serialization.json.Json +import kotlinx.serialization.json.encodeToStream import org.fossify.commons.activities.ManageBlockedNumbersActivity import org.fossify.commons.dialogs.* import org.fossify.commons.extensions.* @@ -118,6 +118,7 @@ class SettingsActivity : SimpleActivity() { } } + @OptIn(kotlinx.serialization.ExperimentalSerializationApi::class) private fun exportMessages(uri: Uri) { ensureBackgroundThread { var success = false @@ -128,11 +129,8 @@ class SettingsActivity : SimpleActivity() { return@getMessagesToExport } val json = Json { encodeDefaults = true } - val jsonString = json.encodeToString(messagesToExport) - val outputStream = contentResolver.openOutputStream(uri)!! - - outputStream.use { - it.write(jsonString.toByteArray()) + contentResolver.openOutputStream(uri)!!.buffered().use { outputStream -> + json.encodeToStream(messagesToExport, outputStream) } success = true toast(org.fossify.commons.R.string.exporting_successful) diff --git a/app/src/main/kotlin/org/fossify/messages/helpers/MessagesImporter.kt b/app/src/main/kotlin/org/fossify/messages/helpers/MessagesImporter.kt index 6f78feb2..476c2b80 100644 --- a/app/src/main/kotlin/org/fossify/messages/helpers/MessagesImporter.kt +++ b/app/src/main/kotlin/org/fossify/messages/helpers/MessagesImporter.kt @@ -4,6 +4,7 @@ import android.net.Uri import android.util.Xml import kotlinx.serialization.SerializationException import kotlinx.serialization.json.Json +import kotlinx.serialization.json.decodeFromStream import org.fossify.commons.extensions.showErrorToast import org.fossify.commons.extensions.toast import org.fossify.commons.helpers.ensureBackgroundThread @@ -37,13 +38,12 @@ class MessagesImporter(private val activity: SimpleActivity) { } } + @OptIn(kotlinx.serialization.ExperimentalSerializationApi::class) private fun importJson(uri: Uri) { try { - val jsonString = activity.contentResolver.openInputStream(uri)!!.use { inputStream -> - inputStream.bufferedReader().readText() + val deserializedList = activity.contentResolver.openInputStream(uri)!!.buffered().use { inputStream -> + Json.decodeFromStream>(inputStream) } - - val deserializedList = Json.decodeFromString>(jsonString) if (deserializedList.isEmpty()) { activity.toast(org.fossify.commons.R.string.no_entries_for_importing) return