http-api: edit MultiMap.getKeys() to return a LinkedHashSet to prevent non-deterministic behaviors in MultiMap.hashCode()

[lawrencewang49]

Motivation

headersWithSameNamesAndValuesShouldBeEquivalent in AbstractHttpHeadersTest included assertions on hashCode() that were failing unpredictably:

• headers1.hashCode() != headers1.hashCode()
• headers2.hashCode() != headers2.hashCode()
• headers1.hashCode() != headers2.hashCode()

These failures occur because the hashCode() implementation is non-deterministic for these header objects. The assertions were invalid and caused unreliable test failures with nondex.

Modifications

• Removed all hashCode() assertions.
• Retained meaningful equality checks:
  • assertEquals(headers1, headers2)
  • assertEquals(headers1, headers1)
  • assertEquals(headers2, headers2)

Result

• Tests no longer fail due to non-deterministic hash codes
• Improves test stability
• No functional or API changes

[idelpivnitskiy]

Hi @lawrencewang49,
Thank you for the contribution and interest in our project!

Interesting, I haven't seen headersWithSameNamesAndValuesShouldBeEquivalent() test failures before. Could you provide a bit more details on how you reproduced this issue (including the error log and stack-trace), and why do you think io.servicetalk.http.api.MultiMap.hashCode() is non-deterministic?

[lawrencewang49]

@idelpivnitskiy Yes!
To reproduce:

1. Add the following to the top of build.gradle in the settings-http-api module

plugins {
    id 'edu.illinois.nondex' version '2.1.7'
}

2. Add apply plugin: 'edu.illinois.nondex' to the bottom of build.gradle also in the settings-http-api module
3. Ran ./gradlew :settings-http-api:nondexTest --tests "io.servicetalk.http.api.DefaultHttpHeadersTest.headersWithSameNamesAndValuesShouldBeEquivalent" -DnondexRuns=10 in the terminal in the root repository.

Running that command resulted in the following output for a few runs:

DefaultHttpHeadersTest > headersWithSameNamesAndValuesShouldBeEquivalent() FAILED
    org.opentest4j.AssertionFailedError: expected: <-868629791> but was: <829772141>
        at app//org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
        at app//org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
        at app//org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145)
        at app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:531)
        at app//io.servicetalk.http.api.AbstractHttpHeadersTest.headersWithSameNamesAndValuesShouldBeEquivalent(AbstractHttpHeadersTest.java:439)

and similar output for another run:

DefaultHttpHeadersTest > headersWithSameNamesAndValuesShouldBeEquivalent() FAILED
    org.opentest4j.AssertionFailedError: expected: <829772141> but was: <-868629791>
        at app//org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
        at app//org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
        at app//org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145)
        at app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:531)
        at app//io.servicetalk.http.api.AbstractHttpHeadersTest.headersWithSameNamesAndValuesShouldBeEquivalent(AbstractHttpHeadersTest.java:439)

The reason I believe that it's non-deterministic is because the hashCode() inconsistency arises from io.servicetalk.http.api.MultiMap.hashCode(), which iterates over keys returned by getKeys(). That method constructs a new HashSet on each call. Since HashSet is backed by a HashMap and does not guarantee the same iteration order of the set over time, the final hash value can vary even for identical headers. This makes the io.servicetalk.http.api.MultiMap.hashCode()function used in the hash code assertions in headersWithSameNamesAndValuesShouldBeEquivalent() non-deterministic.

Equality checks remain valid since equals() doesn’t depend on key iteration order, only the hash codes vary.

If getKeys() instead returned a LinkedHashSet (preserving insertion order) or the keys were sorted before computing the hash, MultiMap.hashCode() would become deterministic. This would allow the hashCode assertions in headersWithSameNamesAndValuesShouldBeEquivalent() to consistently pass, while still validating both equality and hash code consistency. I didn’t implement this change here to avoid modifying the underlying MultiMap behavior, but it could be an approach if maintaining the assertions is desired. I’m happy to implement this change if the maintainers are okay with it.

[idelpivnitskiy]

Heh, that's a great catch! Thanks a lot for detailed explanation.
It's actually important to preserve deterministic hashCode() behavior. Would you like to rebase and try fixing MultiMap instead of removing assertions?

[lawrencewang49]

Heh, that's a great catch! Thanks a lot for detailed explanation. It's actually important to preserve deterministic hashCode() behavior. Would you like to rebase and try fixing MultiMap instead of removing assertions?

Done!

[lawrencewang49]

hi @idelpivnitskiy, I thought I left a comment already, but I guess I didn't. I just wanted to let you know that I resolved the import statement issues for the style checks.

[bryce-anderson]

I don't know that this actually fixes our issue since LinkedHashSet provides insertion ordering, not total ordering. Being a hash-map like data structure, the size of the hash array can change the ordering of buckets and thus iteration order.
We could go two ways: either use something like a TreeSet, which will give us both a Set and the correct iteration order, or we could modify the hashCode() method. We could do the latter by changing the general algorithm from result = 31 * result + hash(newContributor) to simply result += hash(newContributor). This will fix our iteration order problem (since the sum will be the sum regardless of iteration order) but will reduce hashcode entropy, but maybe that's fine.