Fix deadlock in AbstractRequestCache.requests() due to unstable hashCode#12166
Open
gnodet wants to merge 1 commit into
Open
Fix deadlock in AbstractRequestCache.requests() due to unstable hashCode#12166gnodet wants to merge 1 commit into
gnodet wants to merge 1 commit into
Conversation
The `requests()` method uses a HashMap to coordinate batch results between the batch supplier and the individual CachingSupplier instances. When a request object's hashCode() changes between HashMap.put() and HashMap.containsKey() (e.g., because ResolverRequest includes a RequestTrace with mutable ModelBuilderRequest data), the key is stored in one bucket but looked up in another, causing containsKey() to return false. The individualSupplier then waits forever for a result that is already in the map but unreachable. Switch to IdentityHashMap which uses System.identityHashCode() and reference equality (==), both of which are stable regardless of mutable object state. This is safe because the code always uses the same object reference for put and get. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AbstractRequestCache.requests()caused by request objects with unstablehashCode(), by switching the internal coordination map fromHashMaptoIdentityHashMapRoot Cause
The
requests()method uses a localHashMap<REQ, Object>(nonCachedResults) to coordinate batch resolution results. The batch supplier puts resolved results into this map, and theindividualSupplierlambda retrieves them viacontainsKey().The problem:
ResolverRequestis a Java record whosehashCode()transitively includesRequestTrace.data, which holds aModelBuilderRequestcontaining mutablesystemProperties. During artifact resolution, these properties can change, causing the same object to produce different hash values atput()time vscontainsKey()time. The entry is stored in one hash bucket but looked up in another —containsKey()returnsfalse, and the thread waits forever on a result that is already in the map.Debug evidence (same object reference, three different hashes at different times):
Fix
Switch from
HashMaptoIdentityHashMap, which usesSystem.identityHashCode()(stable, based on object identity) and==for equality. Since the code always operates on the same object references for put and get (confirmed bysameRef=true), this is safe and correct.Impact
This fixes the ~80 timeout failures observed in Maven 4 compatibility testing where Maven 4 hangs indefinitely on projects using
maven-remote-resources-plugin. The hang occurs during parent POM resolution triggered by the plugin'sgetProjects()call, when the model builder's request trace mutates system properties mid-resolution.Test plan
httpcomponents-stylecheckhangs indefinitely with Maven 4 + mvnup (addsmaven-remote-resources-plugin:3.0.0)mvn verify -DskipTests) passes with the fixClaude Code on behalf of Guillaume Nodet