RFR: 8003417: WeakHashMap$HashIterator removes wrong entry
Roger Riggs
rriggs at openjdk.java.net
Mon Dec 6 18:16:15 UTC 2021
On Sat, 20 Nov 2021 10:08:41 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:
> Can I please get a review for this change which proposes to fix the issue reported in https://bugs.openjdk.java.net/browse/JDK-8003417?
>
> The issue notes that this is applicable for `WeakHashMap` which have `null` keys. However, the issue is even applicable for `WeakHashMap` instances which don't have `null` keys, as reproduced and shown by the newly added jtreg test case in this PR.
>
> The root cause of the issue is that once the iterator is used to iterate till the end and the `remove()` is called, then the `WeakHashMap$HashIterator#remove()` implementation used to pass `null` as the key to remove from the map, instead of the key of the last returned entry. The commit in this PR fixes that part.
>
> A new jtreg test has been added which reproduces the issue as well as verifies the fix.
> `tier1` testing and this new test have passed after this change. However, I guess this will require a JCK run to be run too, perhaps? If so, I will need help from someone who has access to them to have this run against those please.
The value of `currentKey` should track the value of `lastReturned.get()` and does so in the `nextEntry()` and `remove()` methods.
The odd statement is in `hasNext` that set the state of `currentkey` to null independently of `lastReturned`.
Leaving it non-null will allow `remove` to operate correctly but may have a downside of keeping a strong reference to last key until the HashIterator is GC'd.
The `remove()` method can correctly use `currentKey` to remove the last entry returned.
-------------
Changes requested by rriggs (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/6488
More information about the core-libs-dev
mailing list