On Mon, 25 Oct 2021 09:21:28 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

>> Can I please get a review for this change which fixes the issue reported in 
>> https://bugs.openjdk.java.net/browse/JDK-8275509?
>> 
>> The `ModuleDescriptor.hashCode()` method uses the hash code of its various 
>> components to compute the final hash code. While doing so it ends up calling 
>> hashCode() on (collection of) various `enum` types. Since the hashCode() of 
>> enum types is generated from a call to `java.lang.Object.hashCode()`, their 
>> value isn't guaranteed to stay the same across multiple JVM runs.
>> 
>> The commit here proposes to use the ordinal of the enum as the hash code. As 
>> Alan notes in the mailing list discussion, any changes to the ordinal of the 
>> enum (either due to addition of new value, removal of a value or just 
>> reordering existing values, all of which I think will be rare in these 
>> specific enum types) isn't expected to produce the same hash code across 
>> those changed runtimes and that should be okay. 
>> 
>> The rest of the ModuleDescriptor.hashCode() has been reviewed too and apart 
>> from calls to the enum types hashCode(), rest of the calls in that 
>> implementation, either directly or indirectly end up as calls on 
>> `java.lang.String.hashCode()` and as such aren't expected to cause 
>> non-deterministic values.
>> 
>> A new jtreg test has been added to reproduce this issue and verify the fix.
>
> Jaikiran Pai has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   add newline at end of test class

Hello Alan,

>  I'd prefer not have commented out "TODO" code if possible.

Done. I've updated the PR to remove that TODO.

> I think the test will need a round or two of clean-up to get it more 
> consistent
with the existing naming and style of tests in this area.?

Do you mean the jtreg `driver` style this test is using? I used it because it 
was necessary to compare output (the hashCode value) between multiple JVM runs. 
I use the `driver` along with the `ProcessBuilder`  to capture the output 
between multiple JVM runs and compare those.

-------------

PR: https://git.openjdk.java.net/jdk/pull/6078

Reply via email to