Repository navigation
Clear removed values in IntMap - #1537
Merged
Merged
Conversation
IntMap.remove and the remove method of its iterators cleared the key of a removed entry but not its value. The map kept a reference to the value, containsValue still found it, findKey returned 0 for it, and after key 0 was removed with an iterator, put(0, value) returned it as the old value. Also, get(0, defaultValue) returned null instead of the default value if the map had no key 0, since get was inlined in EsotericSoftware#876.
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved issues were identified.
No open issues 🎉
What changed in this PR
Fixes IntMap stale-value retention and restores default handling for absent key 0.
Changes:
- Clears values during direct and iterator removals.
- Restores the supplied default for absent key
0. - Adds regression tests for both behaviors.
| File | Description |
|---|---|
test/com/esotericsoftware/kryo/util/IntMapTest.java |
Adds regression coverage for removals and default values. |
src/com/esotericsoftware/kryo/util/IntMap.java |
Corrects removal cleanup and default lookup behavior. |
Review effort: Lite
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.
Fixes #1536.
IntMap.removeand theremovemethod of its iterators cleared the key of a removed entry but not its value. So the map kept a reference to the value, eg to theRegistrationremoved byDefaultClassResolver.unregister,containsValuestill found it,findKeyreturned 0 for it, and after key 0 was removed with an iterator,put(0, value)returned it as the old value. The value is now set to null, like inObjectMapand in libGDX'sIntMap, which has the same implementation (libgdx/libgdx#6285).get(0, defaultValue)returned null instead of the default value if the map had no key 0, sincegetwas inlined in #876. It returns the default value again.IntMapTestchecks both. It fails when any of the four changes is reverted.mvn -pl main testpasses on JDK 8 (built with JDK 11), 11, 17, 21 and 25, andmvn -pl main formatter:validatepasses. With the change applied to kryo-6, its tests pass on JDK 17, 21 and 25.