From 877b78aeabd5682e1b54005f544c3f9fdc9ccfb3 Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 26 May 2025 15:34:03 +0100 Subject: [PATCH 01/13] change to discardOldestFileIfNeeded sorting --- .../src/main/java/com/bugsnag/android/FileStore.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt index f7ede678cb..d81372aa5f 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt @@ -114,7 +114,10 @@ internal abstract class FileStore( if (isStorageDirValid(storageDir)) { val listFiles = storageDir.listFiles() ?: return if (listFiles.size < maxStoreCount) return - val sortedListFiles = listFiles.sortedBy { it.lastModified() } + // ensures sorting deterministically when files have identical timestamps + val sortedListFiles = listFiles.sortedWith( + compareBy { it.lastModified() }.thenBy { it.name } + ) // Number of files to discard takes into account that a new file may need to be written val numberToDiscard = listFiles.size - maxStoreCount + 1 var discardedCount = 0 From 3f5acae66405a2fbd1e1a7643c2b76a1d850aa31 Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 26 May 2025 15:39:22 +0100 Subject: [PATCH 02/13] update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9958c7a208..1f0519781e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ * Allow the metadata in `leaveBreadcrumb` to be null rather than enforcing non-null, aligning `bugsnag-android` with our other SDKs [#2180](https://github.com/bugsnag/bugsnag-android/pull/2180) +* Added deterministic sorting for `discardOldestFileIfNeeded` method to avoid potential crashes where mutliple files have the same last modified time + [#2181](https://github.com/bugsnag/bugsnag-android/pull/2189) + ## 6.13.0 (2025-04-15) ### Enhancements From 342b8b8527e7e070a83f95116fcbbc4b27a53294 Mon Sep 17 00:00:00 2001 From: Christian Date: Tue, 27 May 2025 12:44:54 +0100 Subject: [PATCH 03/13] cached lastmodified --- .../main/java/com/bugsnag/android/FileStore.kt | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt index d81372aa5f..86e56c1171 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt @@ -114,13 +114,21 @@ internal abstract class FileStore( if (isStorageDirValid(storageDir)) { val listFiles = storageDir.listFiles() ?: return if (listFiles.size < maxStoreCount) return - // ensures sorting deterministically when files have identical timestamps - val sortedListFiles = listFiles.sortedWith( - compareBy { it.lastModified() }.thenBy { it.name } - ) + + // Store lastModified to ensure it doesn't change between sorting + val fileMeta = listFiles.map { file -> + file to file.lastModified() + } + + //sort by cached lastModified timesstamps + val sortedListFiles = fileMeta + .sortedBy { it.second } + .map { it.first } + // Number of files to discard takes into account that a new file may need to be written val numberToDiscard = listFiles.size - maxStoreCount + 1 var discardedCount = 0 + for (file in sortedListFiles) { if (discardedCount == numberToDiscard) { return From ef5784daf995d452f00db7ac6d9694d98d07b65c Mon Sep 17 00:00:00 2001 From: Christian Date: Tue, 27 May 2025 13:33:17 +0100 Subject: [PATCH 04/13] linting --- .../src/main/java/com/bugsnag/android/FileStore.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt index 86e56c1171..41c4b82a44 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt @@ -120,7 +120,7 @@ internal abstract class FileStore( file to file.lastModified() } - //sort by cached lastModified timesstamps + // Sort by cached lastModified timesstamps val sortedListFiles = fileMeta .sortedBy { it.second } .map { it.first } From 455205be4d1cd7c4a1ce3998e91ecdd80652c53b Mon Sep 17 00:00:00 2001 From: Christian Date: Wed, 28 May 2025 10:26:02 +0100 Subject: [PATCH 05/13] Created Comparable class --- .../main/java/com/bugsnag/android/FileStore.kt | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt index 41c4b82a44..7c4bfc7336 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt @@ -117,13 +117,13 @@ internal abstract class FileStore( // Store lastModified to ensure it doesn't change between sorting val fileMeta = listFiles.map { file -> - file to file.lastModified() + FileWithTimestamp(file, file.lastModified()) } // Sort by cached lastModified timesstamps val sortedListFiles = fileMeta - .sortedBy { it.second } - .map { it.first } + .sorted() + .map(FileWithTimestamp::file) // Number of files to discard takes into account that a new file may need to be written val numberToDiscard = listFiles.size - maxStoreCount + 1 @@ -199,3 +199,14 @@ internal abstract class FileStore( } } } +/** + * A data holder for associating a {@link File} with its last modified timestamp. + * + * @param file The file to associate with a timestamp. + * @param timestamp The last modified time of the file, cached to ensure consistent ordering. + */ +private data class FileWithTimestamp(val file: File, val timestamp: Long) : Comparable { + override fun compareTo(other: FileWithTimestamp): Int { + return timestamp.compareTo(other.timestamp) + } +} \ No newline at end of file From c2e162955cc386f33bb365d3a3b7ad0a8b1b7d80 Mon Sep 17 00:00:00 2001 From: Christian Date: Wed, 28 May 2025 10:30:40 +0100 Subject: [PATCH 06/13] linting --- .../src/main/java/com/bugsnag/android/FileStore.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt index 7c4bfc7336..6562e32e97 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt @@ -199,6 +199,7 @@ internal abstract class FileStore( } } } + /** * A data holder for associating a {@link File} with its last modified timestamp. * @@ -209,4 +210,4 @@ private data class FileWithTimestamp(val file: File, val timestamp: Long) : Comp override fun compareTo(other: FileWithTimestamp): Int { return timestamp.compareTo(other.timestamp) } -} \ No newline at end of file +} From f085e4b8e5380caf4ca0435cb7b77dc4d8872207 Mon Sep 17 00:00:00 2001 From: Christian Rafferty <38156512+clr182@users.noreply.github.com> Date: Tue, 3 Jun 2025 08:52:19 +0100 Subject: [PATCH 07/13] Update CHANGELOG.md Co-authored-by: Jason --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f0519781e..1081bdcc6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ * Allow the metadata in `leaveBreadcrumb` to be null rather than enforcing non-null, aligning `bugsnag-android` with our other SDKs [#2180](https://github.com/bugsnag/bugsnag-android/pull/2180) -* Added deterministic sorting for `discardOldestFileIfNeeded` method to avoid potential crashes where mutliple files have the same last modified time +* Added deterministic sorting for `discardOldestFileIfNeeded` method to avoid potential crashes when files are being written while sorting the queue [#2181](https://github.com/bugsnag/bugsnag-android/pull/2189) ## 6.13.0 (2025-04-15) From fb472e5a38f735f3f816f9f3bfbc8cb14db95458 Mon Sep 17 00:00:00 2001 From: Christian Rafferty <38156512+clr182@users.noreply.github.com> Date: Tue, 3 Jun 2025 08:52:43 +0100 Subject: [PATCH 08/13] Update bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt Co-authored-by: Jason --- .../src/main/java/com/bugsnag/android/FileStore.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt index 6562e32e97..7644541b4f 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt @@ -116,7 +116,7 @@ internal abstract class FileStore( if (listFiles.size < maxStoreCount) return // Store lastModified to ensure it doesn't change between sorting - val fileMeta = listFiles.map { file -> + val timestampedFiles = listFiles.mapTo(ArrayList(listFiles.size)) { file -> FileWithTimestamp(file, file.lastModified()) } From 57eee67ef4bd7b3d5e6eeb9ea1ac84563ab580b8 Mon Sep 17 00:00:00 2001 From: Christian Rafferty <38156512+clr182@users.noreply.github.com> Date: Tue, 3 Jun 2025 08:52:57 +0100 Subject: [PATCH 09/13] Update bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt Co-authored-by: Jason --- .../src/main/java/com/bugsnag/android/FileStore.kt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt index 7644541b4f..059ccb2d1f 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt @@ -121,9 +121,7 @@ internal abstract class FileStore( } // Sort by cached lastModified timesstamps - val sortedListFiles = fileMeta - .sorted() - .map(FileWithTimestamp::file) + timestampedFiles.sort() // Number of files to discard takes into account that a new file may need to be written val numberToDiscard = listFiles.size - maxStoreCount + 1 From 6510dd617f3dd30ee960ba1887b1754905cfde08 Mon Sep 17 00:00:00 2001 From: Christian Date: Tue, 3 Jun 2025 09:25:25 +0100 Subject: [PATCH 10/13] fixed unresolved reference --- .../src/main/java/com/bugsnag/android/FileStore.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt index 059ccb2d1f..6f000d33c0 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt @@ -127,7 +127,7 @@ internal abstract class FileStore( val numberToDiscard = listFiles.size - maxStoreCount + 1 var discardedCount = 0 - for (file in sortedListFiles) { + for (file in timestampedFiles.map { it.file }) { if (discardedCount == numberToDiscard) { return } else if (!queuedFiles.contains(file)) { From 9f849b74bce00315962395bcbac8e3cf0ae8c7d9 Mon Sep 17 00:00:00 2001 From: Christian Date: Tue, 3 Jun 2025 09:27:43 +0100 Subject: [PATCH 11/13] avoid unnecessary .map { it.file} --- .../src/main/java/com/bugsnag/android/FileStore.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt index 6f000d33c0..04fb2afe7f 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt @@ -127,7 +127,8 @@ internal abstract class FileStore( val numberToDiscard = listFiles.size - maxStoreCount + 1 var discardedCount = 0 - for (file in timestampedFiles.map { it.file }) { + for (file in timestampedFiles) { + val file = fileMeta.file if (discardedCount == numberToDiscard) { return } else if (!queuedFiles.contains(file)) { From 035f17e6734beb7f9ba66994f35874e435e21475 Mon Sep 17 00:00:00 2001 From: Christian Date: Tue, 3 Jun 2025 09:58:10 +0100 Subject: [PATCH 12/13] unresolved ref --- .../src/main/java/com/bugsnag/android/FileStore.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt index 04fb2afe7f..1643ef777f 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt @@ -127,7 +127,7 @@ internal abstract class FileStore( val numberToDiscard = listFiles.size - maxStoreCount + 1 var discardedCount = 0 - for (file in timestampedFiles) { + for (fileMeta in timestampedFiles) { val file = fileMeta.file if (discardedCount == numberToDiscard) { return From 83693ee8ece09a1e77d62e69e31f6e5296915b64 Mon Sep 17 00:00:00 2001 From: Christian Rafferty <38156512+clr182@users.noreply.github.com> Date: Tue, 10 Jun 2025 10:54:27 +0100 Subject: [PATCH 13/13] Update bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt Co-authored-by: Jason --- .../src/main/java/com/bugsnag/android/FileStore.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt index 1643ef777f..4c76b54dad 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt @@ -115,7 +115,7 @@ internal abstract class FileStore( val listFiles = storageDir.listFiles() ?: return if (listFiles.size < maxStoreCount) return - // Store lastModified to ensure it doesn't change between sorting + // Store lastModified to ensure it doesn't change during sort val timestampedFiles = listFiles.mapTo(ArrayList(listFiles.size)) { file -> FileWithTimestamp(file, file.lastModified()) }