On Wed, 3 Sep 2025 05:51:03 GMT, Jaikiran Pai <[email protected]> wrote:
>> Can I please get a review of this test-only change which addresses a failure >> in `test/jdk/java/util/zip/ZipFile/ZipSourceCache.java` test? >> >> As noted in the description of https://bugs.openjdk.org/browse/JDK-8366439, >> this test fails if the underlying file system's timestamp granularity >> doesn't allow for the last modified timestamp to increase when the test's >> ZIP file is updated with newer content. >> >> The change in this PR updates the test to first check that the last modified >> timestamp has increased on the ZIP file that was updated, before running the >> rest of the test assertions. The test continues to pass in our CI with this >> change. > > Hello Sean, > >> thanks for looking at this @jaikiran - Do we have a product regression here >> with this env ? > > I don't think this is a product bug. > >> The Source.hashCode() wasn't able to differentiate on the updated file due >> to to crude timestamps being returned from the buggy filesystem ? > > The `java.util.zip.ZipFile` maintains an internal cache for an underlying ZIP > file to read its CEN just once to try and avoid reading it every time a > `ZipFile` instance is created. One internal implementation detail of that > cache is that it has the ability to detect an update to the underlying ZIP > file so that when a subsequent instance of `ZipFile` is created then it won't > re-use an already cached CEN but instead will re-read the CEN (and cache it) > afresh. To detect the updates to the ZIP file, it uses the last modified time > of the file; and last modified timestamps have a granularity. > > My understanding is that filesystems are allowed to have different > granularity of timestamps. In fact `java.io.File.lastModified()` has this API > note: > > * @apiNote > * While the unit of time of the return value is milliseconds, the > * granularity of the value depends on the underlying file system and may > * be larger. For example, some file systems use time stamps in units of > * seconds. > > So it isn't a bug in the filesystem. Nor is it a bug in the `ZipFile` > implementation's cache. In fact, in this test, if you introduce a delay of at > least a second from the time ZIP file was modified to the time you created a > new instance of `ZipFile`, the update would be noticed due to the last > modified time changing and the CEN would be re-read afresh with a new entry > in the internal cache. Hello @jaikiran I'm wondering if it would be better/simpler to manually update the last modification time of the first file created into the past post-creation, to avoid these granularity issues. Something like this after the first `createZipFile` call in `setup`: createZipFile("test1"); // Avoid file system granularity issues causing last modified time clash long oneMinuteAgo = absoluteFile.lastModified() - 60_000; absoluteFile.setLastModified(oneMinuteAgo); ------------- PR Comment: https://git.openjdk.org/jdk/pull/27005#issuecomment-3247859215
