RFR: 8003417: WeakHashMap$HashIterator removes wrong entry

[Stuart Marks]

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.

I've put extensive comments and explanation into the bug report. I put them there instead of here because they're more closely related to the test case in that bug report, and they also include general statements about WeakHashMap iteration, not specific to the change proposed here.

TL;DR I don't think that changing remove to `remove(lastReturned.get())` is the correct fix. It requires revisting the invariants maintained by `HashIterator`. It might be as simple as removing the clearing of `currentKey` from `hasNext()`, as mentioned in the bug report, but this requires more thought.

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

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