Unidata / netcdf-java

The Unidata netcdf-java library
https://docs.unidata.ucar.edu/netcdf-java/current/userguide/index.html
BSD 3-Clause "New" or "Revised" License
142 stars 69 forks source link

Fix disk cache cleanup sort errors caused by files being altered during sorting #1238

Closed okey closed 11 months ago

okey commented 11 months ago

Description of Changes

Java versions newer than 6 check that the sort invariant is maintained. If a cached file is modified during sort, then the modified time changes, violating the invariant and causing an exception to be thrown. Previously, this would have silently caused the resulting sort to be incorrect.

Neither of these results are ideal, but throwing an exception causes the cache cleanup to fail, which can lead to unexpectedly high cache usage.

A quick and nasty hack to revert to Java 6 behaviour is to insert System.setProperty("java.util.Arrays.useLegacyMergeSort", "true"); somewhere, but it is better to fix the sort completely.

This PR caches the file modification times and looks them up during the sort.

PR Checklist

CLAassistant commented 11 months ago

CLA assistant check
All committers have signed the CLA.

haileyajohnson commented 11 months ago

Thanks for the contribution, @okey! I refactored your solution a bit to still plug in a comparator, but one that uses a static lastModified property. As for cleanCache(long maxBytes, Comparator<File> fileComparator, StringBuilder sbuff), I don't think we want to deprecate it since there are still reasons users may want to sub their own Comparator to sort the files in their cache.

okey commented 11 months ago

@haileyajohnson Thanks for the refactor, if you're happy with using a static property then I think it's a good solution. ~Do you want to fix the test failures or shall I?~