[jira] [Comment Edited] (COLLECTIONS-802) ReferenceMap iterator remove violates contract

["Ng Tsz Sum (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/COLLECTIONS-802?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17526507#comment-17526507 ] 

Ng Tsz Sum edited comment on COLLECTIONS-802 at 4/22/22 3:58 PM:
-----------------------------------------------------------------

runnable with java 8, same result as stated
{code:java}
    @Test
    public void iterator_remove_failed_after_hasNext() {
        ReferenceMap<Integer, Integer> map = new ReferenceMap<>();
        map.put(1, 2);
        Iterator<Map.Entry<Integer, Integer>> iter = map.entrySet().iterator();
        assertTrue(iter.hasNext());
        assertTrue(iter.hasNext());
        assertEquals(iter.next().getValue(), new Integer(2));
        // comment below hasNext() call will pass
        assertFalse(iter.hasNext());
        iter.remove();
        assertTrue("Expect empty but have entry: " + map.toString(), map.isEmpty());
    }
{code}
Suspect to be due to line 806 and 807 setting currentKey and currentValue to null. 
https://github.com/apache/commons-collections/blob/0b365e4c1833bf55648ca34b9dffd966c31cc69b/src/main/java/org/apache/commons/collections4/map/AbstractReferenceMap.java#L806


was (Author: samabcde):
runnable with java 8, same result as stated
{code:java}
    @Test
    public void iterator_remove_failed_after_hasNext() {
        ReferenceMap<Integer, Integer> map = new ReferenceMap<>();
        map.put(1, 2);
        Iterator<Map.Entry<Integer, Integer>> iter = map.entrySet().iterator();
        assertTrue(iter.hasNext());
        assertTrue(iter.hasNext());
        assertEquals(iter.next().getValue(), new Integer(2));
        // comment below hasNext() call will pass
        assertFalse(iter.hasNext());
        iter.remove();
        assertTrue("Expect empty but have entry: " + map.toString(), map.isEmpty());
    }
{code}


> ReferenceMap iterator remove violates contract
> ----------------------------------------------
>
>                 Key: COLLECTIONS-802
>                 URL: https://issues.apache.org/jira/browse/COLLECTIONS-802
>             Project: Commons Collections
>          Issue Type: Bug
>          Components: Map
>    Affects Versions: 4.4
>            Reporter: Ben Manes
>            Priority: Minor
>         Attachments: ApacheMapTest.java
>
>
> Out of curiosity I ran Guava's testlib Map tests against the Apache types. This uncovered a contract bug where {{Iterator.remove()}} is invalidated by {{{}hasNext(){}}}, causing its call to no-op due to {{currentKey}} becoming {{{}null{}}}. The isolates case is,
> {code:java}
> @Test
> public void iterator_remove() {
>   var map = new ReferenceMap<>();
>   map.put(1, 2);
>   var iter = map.entrySet().iterator();
>   assertTrue(iter.hasNext());
>   assertTrue(iter.hasNext());
>   assertEquals(iter.next(), 1);
>   assertFalse(iter.hasNext());
>   iter.remove();
>   assertEquals(map, Map.of());
> }{code}
> Guava's [testlib|https://github.com/google/guava/tree/master/guava-testlib] has good coverage for the Collections Framework and might be worth integrating. The simple test case that I wrote is attached.



--
This message was sent by Atlassian Jira
(v8.20.7#820007)