Skip to content

Change to discardOldestFileIfNeeded sorting#2189

Merged
lemnik merged 13 commits intonextfrom
Crash-in-discardOldestFileIfNeeded
Aug 14, 2025
Merged

Change to discardOldestFileIfNeeded sorting#2189
lemnik merged 13 commits intonextfrom
Crash-in-discardOldestFileIfNeeded

Conversation

@clr182
Copy link
Copy Markdown
Contributor

@clr182 clr182 commented May 26, 2025

Goal

Fix rare "Comparison method violates its general contract!" crashes in discardOldestFileIfNeeded when the file timestamps change during the sort().

Design

Capture all of the lastModified() timestamps into a stable list before sorting.

Testing

Relied on existing tests

@clr182 clr182 requested review from YYChen01988 and lemnik as code owners May 26, 2025 14:37
@bugsnagbot
Copy link
Copy Markdown
Collaborator

bugsnagbot commented May 26, 2025

Android notifier sizes

Format Size impact of Bugsnag (kB) Size impact of Bugsnag when Minified (kB)
APK 1828.3 1630.42
arm64_v8a 639.23 438.53
armeabi_v7a 581.9 381.19
x86 708.85 512.24
x86_64 684.28 483.58

Generated by 🚫 Danger

Comment thread CHANGELOG.md Outdated
Comment thread bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt Outdated
Comment thread bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt Outdated
@clr182 clr182 requested a review from lemnik June 3, 2025 07:53
Comment thread bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt Outdated
@clr182 clr182 requested a review from lemnik June 10, 2025 10:20
@clr182
Copy link
Copy Markdown
Contributor Author

clr182 commented Jul 15, 2025

Hey @lemnik
wondering if you've had a chance to consider this further? FYI as far as I am aware it hasn't been reported again by any other user.

@lemnik lemnik force-pushed the Crash-in-discardOldestFileIfNeeded branch 2 times, most recently from 516eaa6 to 4d22cc9 Compare August 13, 2025 14:04
@lemnik lemnik force-pushed the Crash-in-discardOldestFileIfNeeded branch from 4d22cc9 to d57176c Compare August 14, 2025 08:51
Copy link
Copy Markdown
Contributor

@YYChen01988 YYChen01988 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lemnik lemnik merged commit e5cfc9d into next Aug 14, 2025
35 checks passed
@lemnik lemnik deleted the Crash-in-discardOldestFileIfNeeded branch August 14, 2025 09:12
@YYChen01988 YYChen01988 mentioned this pull request Aug 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants