Skip to content

Commit e5cfc9d

Browse files
clr182lemnik
andauthored
Change to discardOldestFileIfNeeded sorting (#2189)
* change to discardOldestFileIfNeeded sorting * update changelog * cached lastmodified * linting * Created Comparable class * linting * Update bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt Co-authored-by: Jason <lemnik@users.noreply.github.com> * Update bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt Co-authored-by: Jason <lemnik@users.noreply.github.com> * fixed unresolved reference * avoid unnecessary .map { it.file} * unresolved ref * Update bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt Co-authored-by: Jason <lemnik@users.noreply.github.com> * chore(FileStore): moved FileWithTimestamp to inside FileStore, and inverted the `isStorageDirValid` of `discardOldestFileIfNeeded` --------- Co-authored-by: Jason <lemnik@users.noreply.github.com>
1 parent 465b827 commit e5cfc9d

2 files changed

Lines changed: 42 additions & 18 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
[#2235](https://github.com/bugsnag/bugsnag-android/pull/2235)
1414
* Correct the reporting of the loadAddress for native system libraries and JIT frames
1515
[#2244](https://github.com/bugsnag/bugsnag-android/pull/2244)
16+
* Added deterministic sorting for `discardOldestFileIfNeeded` method to avoid potential crashes where mutliple files have the same last modified time
17+
[#2189](https://github.com/bugsnag/bugsnag-android/pull/2189)
1618

1719
## 6.16.0 (2025-07-31)
1820

bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt

Lines changed: 40 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -112,24 +112,32 @@ internal abstract class FileStore(
112112

113113
fun discardOldestFileIfNeeded() {
114114
// Limit number of saved payloads to prevent disk space issues
115-
if (isStorageDirValid(storageDir)) {
116-
val listFiles = storageDir.listFiles() ?: return
117-
if (listFiles.size < maxStoreCount) return
118-
val sortedListFiles = listFiles.sortedBy { it.lastModified() }
119-
// Number of files to discard takes into account that a new file may need to be written
120-
val numberToDiscard = listFiles.size - maxStoreCount + 1
121-
var discardedCount = 0
122-
for (file in sortedListFiles) {
123-
if (discardedCount == numberToDiscard) {
124-
return
125-
} else if (!queuedFiles.contains(file)) {
126-
logger.w(
127-
"Discarding oldest error as stored error limit reached: '" +
128-
file.path + '\''
129-
)
130-
deleteStoredFiles(setOf(file))
131-
discardedCount++
132-
}
115+
if (!isStorageDirValid(storageDir)) return
116+
val listFiles = storageDir.listFiles() ?: return
117+
if (listFiles.size < maxStoreCount) return
118+
119+
// Store lastModified to ensure it doesn't change during sort
120+
val timestampedFiles = listFiles.mapTo(ArrayList(listFiles.size)) { file ->
121+
FileWithTimestamp(file, file.lastModified())
122+
}
123+
124+
// Sort by cached lastModified timestamps
125+
timestampedFiles.sort()
126+
127+
// Number of files to discard takes into account that a new file may need to be written
128+
val numberToDiscard = listFiles.size - maxStoreCount + 1
129+
var discardedCount = 0
130+
131+
for (fileMeta in timestampedFiles) {
132+
val file = fileMeta.file
133+
if (discardedCount == numberToDiscard) {
134+
return
135+
} else if (!queuedFiles.contains(file)) {
136+
logger.w(
137+
"Discarding oldest error as stored error limit reached: '${file.path}'"
138+
)
139+
deleteStoredFiles(setOf(file))
140+
discardedCount++
133141
}
134142
}
135143
}
@@ -188,4 +196,18 @@ internal abstract class FileStore(
188196
lock.unlock()
189197
}
190198
}
199+
200+
/**
201+
* A data holder for associating a {@link File} with its last modified timestamp.
202+
*
203+
* @param file The file to associate with a timestamp.
204+
* @param timestamp The last modified time of the file, cached to ensure consistent ordering.
205+
*/
206+
private data class FileWithTimestamp(
207+
val file: File,
208+
val timestamp: Long
209+
) : Comparable<FileWithTimestamp> {
210+
override fun compareTo(other: FileWithTimestamp): Int =
211+
timestamp.compareTo(other.timestamp)
212+
}
191213
}

0 commit comments

Comments
 (0)