RFR: 8366439: test/jdk/java/util/zip/ZipFile/ZipSourceCache.java fails with AssertionFailedError: expected: <6> but was: <5>

[Eirik Bjørsnøs]

On Wed, 3 Sep 2025 05:51:03 GMT, Jaikiran Pai <jpai at openjdk.org> 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