harmony-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mikhail Markov (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HARMONY-3883) [classlib]WeakHashMap.keySet().toArray() intermittently throws NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Date Thu, 24 May 2007 10:33:16 GMT

    [ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498573
] 

Mikhail Markov commented on HARMONY-3883:
-----------------------------------------

Xiao-Feng, i've found the real problems and prepared the best patch (as i think :-)).
And the code should work without additional patch (H-3883_add_alt2.patch) as if you call hasNext()
method it extracts the next element from the WeakReference so it'll not be GC-collected and
thus next() method will not throw any exceptions if called after hasNext() method returns
true even if GC was called in-between. Here is the code to demonstrate this:
import java.util.*;

public class Test {
    public static void main(String[] args)  throws Exception {
        WeakHashMap map = new WeakHashMap();
        map.put(new Object(), null);
        Iterator iter = map.keySet().iterator();
        //System.out.println("hasNext() = " + iter.hasNext());
        System.gc();
        System.out.println("next = " + iter.next());
    }
}
The last line throws NoSuchElementException but if we uncomment the line with hasNext() invocation
then the last line returns the stored Object (both on RI & Harmony). So hasNext() and
next() methods are consistent between each other and no additional patches is needed.

But i've found inconsistency between hasNext() and next() methods in Harmony. Here is the
test reproducing the problem:
import java.util.*;

public class Test {
    public static void main(String[] args)  throws Exception {
        WeakHashMap map = new WeakHashMap();
        MyClass cl = new MyClass(2);
        map.put(new MyClass(1), null);
        map.put(cl, null);
        map.put(new MyClass(3), null);
        Iterator iter = map.keySet().iterator();
        System.out.println("next = " + iter.next());
        System.out.println("next = " + iter.next());
        System.gc();
        System.out.println("hasNext = " + iter.hasNext());
    }

    static class MyClass {
        int id = 0;

        public MyClass(int id) {
            this.id = id;
        }

        public int hashCode() {
            return 0;
        }

        public String toString() {
            return "MyClass[id=" + id + "]";
        }
    }
}
As a result of last hasNext() call RI returns false while Harmony - true (while next() methdo
will throw NoSuchElementException on both implementations).


Could you please revert the last commit (H-3883_add_alt2.patch) and re-open this JIRA so i
could add the final patch?
Thanks!
 

> [classlib]WeakHashMap.keySet().toArray() intermittently throws NoSuchElementException
in java.lang.ThreadTest of DRLVM kernel test
> ----------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HARMONY-3883
>                 URL: https://issues.apache.org/jira/browse/HARMONY-3883
>             Project: Harmony
>          Issue Type: Bug
>          Components: Classlib
>         Environment: win/linux 32/64
>            Reporter: Li-Gang Wang
>         Attachments: H-3883.patch, H-3883_add_alt1.patch, H-3883_add_alt2.patch, WeakHashmapTest.java
>
>
> I found there is a design bug in WeakHashMap.keySet().toArray() .
> WeakHashMap.keySet().toArray() inherits AbstractCollection.toArray(). Its implementation
is as follows:
> 	public Object[] toArray() {
> 		int size = size(), index = 0;
> 		Iterator<?> it = iterator();
> 		Object[] array = new Object[size];
> 		while (index < size) {
>             array[index++] = it.next();
>         }
> 		return array;
> 	}
> After assigning 'size()' to 'size', the 'while' loop expects 'size' elements in the iterator
'it'. But actually GC might happen and clear some weak keys of the WeakHashMap instance in
the meantime. The iterator will skip the cleared weak keys in 'it.hasNext()' called by 'it.next()',
so the actual number of the elements in 'it' is smaller than 'size'. When 'it' runs out its
elements, 'it.next()' will throw a NoSuchElementException. Parts of HashIterator implementation
in WeakHashMap is as follows:
>         public boolean hasNext() {
>             if (nextEntry != null) {
>                 return true;
>             }
>             while (true) {
>                 if (nextEntry == null) {
>                     while (position < elementData.length) {
>                         if ((nextEntry = elementData[position++]) != null) {
>                             break;
>                         }
>                     }
>                     if (nextEntry == null) {
>                         return false;
>                     }
>                 }
>                 // ensure key of next entry is not gc'ed
>                 nextKey = nextEntry.get();
>                 if (nextKey != null || nextEntry.isNull) {
>                     return true;
>                 }
>                 nextEntry = nextEntry.next;
>             }
>         }
>         public R next() {
>             if (expectedModCount == modCount) {
>                 if (hasNext()) {
>                     currentEntry = nextEntry;
>                     nextEntry = currentEntry.next;
>                     R result = type.get(currentEntry);
>                     // free the key
>                     nextKey = null;
>                     return result;
>                 }
>                 throw new NoSuchElementException();
>             }
>             throw new ConcurrentModificationException();
>         }
> I suspect the intermittent failure in java.lang.ThreadTest may disappear if this bug
is fixed.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message