Bug 1296750 - Don't crash if multiple files have same timestamp in TelemetryJSONFilePingStore.deleteSmallestFiles r=Grisha

Using a set is bad since it will silently ignore multiple files with the same timestamp, resulting in crashes when
the set has fewer items than we expected. A list is probably the best choice in this instance.

(Note: I am not familiar enough with this code to know whether or not this is an expected situation, i.e.
 whether or not timestamps should be unique.)

MozReview-Commit-ID: 5TRNLgTsSHO

--HG--
extra : rebase_source : eff575ed7ef36555d20ffa568cb80bef1a1dfadc
This commit is contained in:
Andrzej Hunt 2016-08-19 14:18:43 -07:00
parent ec70caf659
commit a6b9e8ba76

View File

@ -126,17 +126,22 @@ public class TelemetryJSONFilePingStore extends TelemetryPingStore {
return;
}
final SortedSet<File> sortedFiles = new TreeSet<>(fileLastModifiedComparator);
sortedFiles.addAll(Arrays.asList(files));
// It's possible that multiple files will have the same timestamp: in this case they are treated
// as equal by the fileLastModifiedComparator. We therefore have to use a sorted list (as
// opposed to a set, or map).
final ArrayList<File> sortedFiles = new ArrayList<>(Arrays.asList(files));
Collections.sort(sortedFiles, fileLastModifiedComparator);
deleteSmallestFiles(sortedFiles, files.length - MAX_PING_COUNT);
}
private void deleteSmallestFiles(final SortedSet<File> files, final int numFilesToRemove) {
private void deleteSmallestFiles(final ArrayList<File> files, final int numFilesToRemove) {
final Iterator<File> it = files.iterator();
int i = 0;
while (i < numFilesToRemove) {
i += 1;
// Sorted set so we're iterating over ascending files.
// Sorted list so we're iterating over ascending files.
final File file = it.next(); // file count > files to remove so this should not throw.
file.delete();
}