[jira] [Commented] (COLLECTIONS-444) SetUniqueList may become inconsistent

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (COLLECTIONS-444) SetUniqueList may become inconsistent

ASF GitHub Bot (Jira)

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

John Vasileff commented on COLLECTIONS-444:
-------------------------------------------

SetUniqueList.set(int index, object), as currently in trunk, never calls "set.remove(removed)" when the new Object equals() the pre-existing Object at the index. However, the Object is removed and re-added to the underlying List by "removed = super.set(index, object)".

As a result, the Set and List may wind up containing different elements per "==". For example:

{code:java}
    Integer a1 = new Integer(1);
    Integer b1 = new Integer(1);
    ...
    lset.clear();
    lset.add(a1); // both Set and List will have a1
    lset.set(0, b1); // List will have b1, Set will have a1
{code}

While normally this won't cause problems, it does seem wrong, and prevents garbage collection of a1 in the example above.

Reverting the patch, and then simply swapping the order of "set.add()" and "set.remove()" may be a better fix for the original problem:

{code:java}
    public E set(final int index, final E object) {
        final int pos = indexOf(object);
        final E removed = super.set(index, object);

        if (pos != -1 && pos != index) {
            // the object is already in the uniq list
            // (and it hasn't been swapped with itself)
            super.remove(pos); // remove the duplicate by index
        }

        set.remove(removed); // remove the item deleted by the set
        set.add(object); // add the new item to the unique set

        return removed; // return the item deleted by the set
    }
{code}
               

> SetUniqueList may become inconsistent
> -------------------------------------
>
>                 Key: COLLECTIONS-444
>                 URL: https://issues.apache.org/jira/browse/COLLECTIONS-444
>             Project: Commons Collections
>          Issue Type: Bug
>          Components: List
>    Affects Versions: 3.2.1
>            Reporter: Thomas Vahrst
>             Fix For: 4.0
>
>
> I found this bug during my work on issue COLLECTIONS-310 :
> When you 'set' an element to a position that contains this element, it is removed from the internal set. This leads to the situation that
> - invocing get() returns the element
> - invocing contains() returns false.
> Extending the existing test method for set:
> {code}
>    public void testSet() {
>         final SetUniqueList<E> lset = new SetUniqueList<E>(new ArrayList<E>(), new HashSet<E>());
>         // Duplicate element
>         final E obj1 = (E) new Integer(1);
>         final E obj2 = (E) new Integer(2);
>         final E obj3 = (E) new Integer(3);
>         lset.add(obj1);
>         lset.add(obj2);
>         lset.set(0, obj1);
>         assertEquals(2, lset.size());
>         assertSame(obj1, lset.get(0));
>         assertSame(obj2, lset.get(1));
>         assertTrue(lset.contains(obj1));  // fails !
>         assertTrue(lset.contains(obj2));
> {code}

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira