You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by "Li-Gang Wang (JIRA)" <ji...@apache.org> on 2007/05/17 08:10:16 UTC
[jira] Created: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
[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
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.
Re: [jira] Commented: (HARMONY-3883) [classlib]WeakHashMap.keySet().toArray() intermittently throws NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by Xiao-Feng Li <xi...@gmail.com>.
To correct the behavior, probably we can loop in the addNext() to find
the first non-NULL value. When a NULL is met, just loop back to get
next entry. Something like:
public boolean addNext() {
if (expectedModCount == modCount) {
while (hasNext()) {
currentEntry = nextEntry;
nextEntry = currentEntry.next;
R result = type.get(currentEntry);
// free the key
if(result == null) continue; // <--- add this line
nextKey = null;
return TRUE;
}
return FALSE
}
throw new ConcurrentModificationException();
}
Note the code is only for example of the idea, not tested.
I am not sure if we want to modify Next() similarly.
Thanks,
xiaofeng
On 5/23/07, Li-Gang Wang (JIRA) <ji...@apache.org> wrote:
>
> [ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498136 ]
>
> Li-Gang Wang commented on HARMONY-3883:
> ---------------------------------------
>
> Mikhail, thanks. I think your two solutions are both ok. And I personally prefer the second one too.
>
> One issue needs to be mentioned. In the latest CC result on Win64, [http://www.harmonytest.org/upload/nstdrlew14_8080.drlvm-test.html], java.lang.ThreadTest.testGetAllStackTraces failed with a NullPointerException. The following is the log:
>
> Test: testGetAllStackTraces
> Class: java.lang.ThreadTest
>
> java.lang.NullPointerException
> at java.lang.ThreadGroup.activeCount(ThreadGroup.java:130)
> at java.lang.ThreadGroup.activeCount(ThreadGroup.java:136)
> at java.lang.Thread.getAllStackTraces(Thread.java:447)
> at java.lang.ThreadTest.testGetAllStackTraces(ThreadTest.java:1011)
> at java.lang.reflect.VMReflection.invokeMethod(VMReflection.java)
> at java.lang.reflect.Method.invoke(Method.java:382)
> at junit.framework.TestCase.runTest(TestCase.java:164)
> at junit.framework.TestCase.runBare(TestCase.java:130)
> at junit.framework.TestResult$1.protect(TestResult.java:106)
> at junit.framework.TestResult.runProtected(TestResult.java:124)
> at junit.framework.TestResult.run(TestResult.java:109)
> at junit.framework.TestCase.run(TestCase.java:120)
> at junit.framework.TestSuite.runTest(TestSuite.java:230)
> at junit.framework.TestSuite.run(TestSuite.java:225)
> at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.run(JUnitTestRunner.java:297)
> at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.launch(JUnitTestRunner.java:672)
> at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.main(JUnitTestRunner.java:546)
>
> The result was produced after H-3883.patch was committed (current implementation). And the failure is not introduced by H-3883.patch.
>
> Xiao-Feng and I think the problem is also in WeakHashMap.keySet().toArray(). In fact there are two areas in toArray(), in which the referent of a weak ref might be cleared. The first is between 'iter.hasNext()' in the 'for' loop in toArray() and 'hasNext()' in iter.next(). The second is between 'hasNext()' in iter.next() and 'type.get(currentEntry)' in iter.next(). In current implementation, if the referent is cleared in the first area, a NoSuchElementException will be thrown. And if it is cleared in the second area, a 'null' will be returned by 'type.get(currentEntry)' and added to Collection. This causes NullPointerException to be thrown.
>
> H-3883_add_alt2.patch eliminates NoSuchElementException, but can not prevent NullPointerException. Please check if I am correct.
>
> > [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.
>
>
--
http://xiao-feng.blogspot.com
Re: [jira] Commented: (HARMONY-3883) [classlib]WeakHashMap.keySet().toArray() intermittently throws NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by Xiao-Feng Li <xi...@gmail.com>.
In that case, the java.lang.ThreadGroup.activeCount() should check if
the thread object is NULL, before invoking its isAlive() method.
Thanks,
xiaofeng
On 5/23/07, Mikhail Markov (JIRA) <ji...@apache.org> wrote:
>
> [ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498255 ]
>
> Mikhail Markov commented on HARMONY-3883:
> -----------------------------------------
>
> Hmm... null entries are allowed in the WeakHashMap. See the following valid code:
> import java.util.WeakHashMap;
>
> public class Test {
> public static void main(String[] args) throws Exception {
> WeakHashMap map = new WeakHashMap();
> map.put("1", null);
> map.put(null, null);
> map.put("2", null);
> Object[] keys = map.keySet().toArray();
>
> for (int i = 0; i < keys.length; ++i) {
> System.out.println("key[" + i + "] = " + keys[i]);
> }
> }
> }
>
> So, the proposed idea does not work as it will remove valid null entry ...
>
> > [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.
>
>
--
http://xiao-feng.blogspot.com
Re: [jira] Commented: (HARMONY-3883) [classlib]WeakHashMap.keySet().toArray() intermittently throws NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by Xiao-Feng Li <xi...@gmail.com>.
Mike, I will commit your patch2, and close this issue, while opening
another JIRA for the java.lang.ThreadGroup.java issue. The goal is to
solve this java.lang.ThreadTest failure finally.
Thanks,
xiaofeng
On 5/24/07, Mikhail Markov (JIRA) <ji...@apache.org> wrote:
>
> [ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498283 ]
>
> Mikhail Markov commented on HARMONY-3883:
> -----------------------------------------
>
> Xio-feng, i guess you've posted your comment to the wrong JIRA :-).
>
> > [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.
>
>
--
http://xiao-feng.blogspot.com
Re: [jira] Updated: (HARMONY-3883) [classlib]WeakHashMap.keySet().toArray() intermittently throws NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by Xiao-Feng Li <xi...@gmail.com>.
Good catch! Thanks, Ligang. The intermittent failure of
java.lang.ThreadTest has been annoying for a long time.
Thanks,
xiaofeng
On 5/17/07, Li-Gang Wang (JIRA) <ji...@apache.org> wrote:
>
> [ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
>
> Li-Gang Wang updated HARMONY-3883:
> ----------------------------------
>
> Attachment: WeakHashmapTest.java
>
> I modified the WeakHashmapTest.java in DRLVM smoke test so that it can easily make this problem appear. Just replace the original one in $drlvm/vm/tests/smoke/stress/WeakHashmapTest.java with the attachment and try to run it individually.
>
> > [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: 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.
>
>
--
http://xiao-feng.blogspot.com
Re: [jira] Commented: (HARMONY-3883) [classlib]WeakHashMap.keySet().toArray() intermittently throws NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by Xiao-Feng Li <xi...@gmail.com>.
To correct the behavior, probably we can loop in the addNext() to find
the first non-NULL value. When a NULL is met, just loop back to get
next entry. Something like:
public boolean addNext() {
if (expectedModCount == modCount) {
while (hasNext()) {
currentEntry = nextEntry;
nextEntry = currentEntry.next;
R result = type.get(currentEntry);
// free the key
if(result == null) continue; // <--- add this line
nextKey = null;
return TRUE;
}
return FALSE
}
throw new ConcurrentModificationException();
}
Note the code is only for example of the idea, not tested.
I am not sure if we want to modify Next() similarly.
Thanks,
xiaofeng
On 5/23/07, Li-Gang Wang (JIRA) <ji...@apache.org> wrote:
>
> [ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498136 ]
>
> Li-Gang Wang commented on HARMONY-3883:
> ---------------------------------------
>
> Mikhail, thanks. I think your two solutions are both ok. And I personally prefer the second one too.
>
> One issue needs to be mentioned. In the latest CC result on Win64, [http://www.harmonytest.org/upload/nstdrlew14_8080.drlvm-test.html], java.lang.ThreadTest.testGetAllStackTraces failed with a NullPointerException. The following is the log:
>
> Test: testGetAllStackTraces
> Class: java.lang.ThreadTest
>
> java.lang.NullPointerException
> at java.lang.ThreadGroup.activeCount(ThreadGroup.java:130)
> at java.lang.ThreadGroup.activeCount(ThreadGroup.java:136)
> at java.lang.Thread.getAllStackTraces(Thread.java:447)
> at java.lang.ThreadTest.testGetAllStackTraces(ThreadTest.java:1011)
> at java.lang.reflect.VMReflection.invokeMethod(VMReflection.java)
> at java.lang.reflect.Method.invoke(Method.java:382)
> at junit.framework.TestCase.runTest(TestCase.java:164)
> at junit.framework.TestCase.runBare(TestCase.java:130)
> at junit.framework.TestResult$1.protect(TestResult.java:106)
> at junit.framework.TestResult.runProtected(TestResult.java:124)
> at junit.framework.TestResult.run(TestResult.java:109)
> at junit.framework.TestCase.run(TestCase.java:120)
> at junit.framework.TestSuite.runTest(TestSuite.java:230)
> at junit.framework.TestSuite.run(TestSuite.java:225)
> at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.run(JUnitTestRunner.java:297)
> at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.launch(JUnitTestRunner.java:672)
> at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.main(JUnitTestRunner.java:546)
>
> The result was produced after H-3883.patch was committed (current implementation). And the failure is not introduced by H-3883.patch.
>
> Xiao-Feng and I think the problem is also in WeakHashMap.keySet().toArray(). In fact there are two areas in toArray(), in which the referent of a weak ref might be cleared. The first is between 'iter.hasNext()' in the 'for' loop in toArray() and 'hasNext()' in iter.next(). The second is between 'hasNext()' in iter.next() and 'type.get(currentEntry)' in iter.next(). In current implementation, if the referent is cleared in the first area, a NoSuchElementException will be thrown. And if it is cleared in the second area, a 'null' will be returned by 'type.get(currentEntry)' and added to Collection. This causes NullPointerException to be thrown.
>
> H-3883_add_alt2.patch eliminates NoSuchElementException, but can not prevent NullPointerException. Please check if I am correct.
>
> > [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.
>
>
--
http://xiao-feng.blogspot.com
Re: [jira] Commented: (HARMONY-3883) [classlib]WeakHashMap.keySet().toArray() intermittently throws NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by Xiao-Feng Li <xi...@gmail.com>.
I've done thread already, strange my action and comments didn't come
to this list.
On 5/24/07, Mikhail Markov <mi...@gmail.com> wrote:
> Xiao-Feng,
>
> Could you please revert the last patch2 and re-open JIRA - i've found the
> real issue and want to post it there.
> (See my comment in JIRA).
>
> Thanks,
> Mikhail
>
>
> On 5/24/07, Xiao-Feng Li (JIRA) <ji...@apache.org> wrote:
> >
> >
> > [
> > https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498449]
> >
> > Xiao-Feng Li commented on HARMONY-3883:
> > ---------------------------------------
> >
> > Mike, I will commit your patch2, and close this issue, while opening
> > another JIRA for the java.lang.ThreadGroup.java issue. The goal is to
> > solve this java.lang.ThreadTest failure finally.
> >
> > Thanks,
> > xiaofeng
> >
> >
> >
> > --
> > http://xiao-feng.blogspot.com
> >
> >
> > > [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.
> >
> >
>
--
http://xiao-feng.blogspot.com
Re: [jira] Commented: (HARMONY-3883) [classlib]WeakHashMap.keySet().toArray() intermittently throws NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by Mikhail Markov <mi...@gmail.com>.
Xiao-Feng,
Could you please revert the last patch2 and re-open JIRA - i've found the
real issue and want to post it there.
(See my comment in JIRA).
Thanks,
Mikhail
On 5/24/07, Xiao-Feng Li (JIRA) <ji...@apache.org> wrote:
>
>
> [
> https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498449]
>
> Xiao-Feng Li commented on HARMONY-3883:
> ---------------------------------------
>
> Mike, I will commit your patch2, and close this issue, while opening
> another JIRA for the java.lang.ThreadGroup.java issue. The goal is to
> solve this java.lang.ThreadTest failure finally.
>
> Thanks,
> xiaofeng
>
>
>
> --
> http://xiao-feng.blogspot.com
>
>
> > [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.
>
>
Re: [jira] Commented: (HARMONY-3883) [classlib]WeakHashMap.keySet().toArray() intermittently throws NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by Xiao-Feng Li <xi...@gmail.com>.
In that case, the java.lang.ThreadGroup.activeCount() should check if
the thread object is NULL, before invoking its isAlive() method.
Thanks,
xiaofeng
On 5/23/07, Mikhail Markov (JIRA) <ji...@apache.org> wrote:
>
> [ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498255 ]
>
> Mikhail Markov commented on HARMONY-3883:
> -----------------------------------------
>
> Hmm... null entries are allowed in the WeakHashMap. See the following valid code:
> import java.util.WeakHashMap;
>
> public class Test {
> public static void main(String[] args) throws Exception {
> WeakHashMap map = new WeakHashMap();
> map.put("1", null);
> map.put(null, null);
> map.put("2", null);
> Object[] keys = map.keySet().toArray();
>
> for (int i = 0; i < keys.length; ++i) {
> System.out.println("key[" + i + "] = " + keys[i]);
> }
> }
> }
>
> So, the proposed idea does not work as it will remove valid null entry ...
>
> > [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.
>
>
--
http://xiao-feng.blogspot.com
Re: [jira] Commented: (HARMONY-3883) [classlib]WeakHashMap.keySet().toArray() intermittently throws NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by Xiao-Feng Li <xi...@gmail.com>.
Mike, I will commit your patch2, and close this issue, while opening
another JIRA for the java.lang.ThreadGroup.java issue. The goal is to
solve this java.lang.ThreadTest failure finally.
Thanks,
xiaofeng
On 5/24/07, Mikhail Markov (JIRA) <ji...@apache.org> wrote:
>
> [ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498283 ]
>
> Mikhail Markov commented on HARMONY-3883:
> -----------------------------------------
>
> Xio-feng, i guess you've posted your comment to the wrong JIRA :-).
>
> > [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.
>
>
--
http://xiao-feng.blogspot.com
[jira] Commented: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Li-Gang Wang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498136 ]
Li-Gang Wang commented on HARMONY-3883:
---------------------------------------
Mikhail, thanks. I think your two solutions are both ok. And I personally prefer the second one too.
One issue needs to be mentioned. In the latest CC result on Win64, [http://www.harmonytest.org/upload/nstdrlew14_8080.drlvm-test.html], java.lang.ThreadTest.testGetAllStackTraces failed with a NullPointerException. The following is the log:
Test: testGetAllStackTraces
Class: java.lang.ThreadTest
java.lang.NullPointerException
at java.lang.ThreadGroup.activeCount(ThreadGroup.java:130)
at java.lang.ThreadGroup.activeCount(ThreadGroup.java:136)
at java.lang.Thread.getAllStackTraces(Thread.java:447)
at java.lang.ThreadTest.testGetAllStackTraces(ThreadTest.java:1011)
at java.lang.reflect.VMReflection.invokeMethod(VMReflection.java)
at java.lang.reflect.Method.invoke(Method.java:382)
at junit.framework.TestCase.runTest(TestCase.java:164)
at junit.framework.TestCase.runBare(TestCase.java:130)
at junit.framework.TestResult$1.protect(TestResult.java:106)
at junit.framework.TestResult.runProtected(TestResult.java:124)
at junit.framework.TestResult.run(TestResult.java:109)
at junit.framework.TestCase.run(TestCase.java:120)
at junit.framework.TestSuite.runTest(TestSuite.java:230)
at junit.framework.TestSuite.run(TestSuite.java:225)
at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.run(JUnitTestRunner.java:297)
at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.launch(JUnitTestRunner.java:672)
at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.main(JUnitTestRunner.java:546)
The result was produced after H-3883.patch was committed (current implementation). And the failure is not introduced by H-3883.patch.
Xiao-Feng and I think the problem is also in WeakHashMap.keySet().toArray(). In fact there are two areas in toArray(), in which the referent of a weak ref might be cleared. The first is between 'iter.hasNext()' in the 'for' loop in toArray() and 'hasNext()' in iter.next(). The second is between 'hasNext()' in iter.next() and 'type.get(currentEntry)' in iter.next(). In current implementation, if the referent is cleared in the first area, a NoSuchElementException will be thrown. And if it is cleared in the second area, a 'null' will be returned by 'type.get(currentEntry)' and added to Collection. This causes NullPointerException to be thrown.
H-3883_add_alt2.patch eliminates NoSuchElementException, but can not prevent NullPointerException. Please check if I am correct.
> [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.
[jira] Commented: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Xiao-Feng Li (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498262 ]
Xiao-Feng Li commented on HARMONY-3883:
---------------------------------------
In that case, the java.lang.ThreadGroup.activeCount() should check if
the thread object is NULL, before invoking its isAlive() method.
Thanks,
xiaofeng
--
http://xiao-feng.blogspot.com
> [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.
[jira] Commented: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Mikhail Markov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498283 ]
Mikhail Markov commented on HARMONY-3883:
-----------------------------------------
Xio-feng, i guess you've posted your comment to the wrong JIRA :-).
> [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.
[jira] Closed: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Xiao-Feng Li (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Xiao-Feng Li closed HARMONY-3883.
---------------------------------
Resolution: Fixed
Assignee: Xiao-Feng Li
> [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
> Assigned To: Xiao-Feng Li
> Attachments: H-3883.patch, H-3883_add_alt1.patch, H-3883_add_alt2.patch, H-3883_add_final.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.
[jira] Commented: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Xiao-Feng Li (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12496814 ]
Xiao-Feng Li commented on HARMONY-3883:
---------------------------------------
The patch shows the bug happens less frequently now. Actually it is not reported in testing, although it may not ultimately eliminate the bug.
> [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, 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.
[jira] Commented: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Xiao-Feng Li (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498449 ]
Xiao-Feng Li commented on HARMONY-3883:
---------------------------------------
Mike, I will commit your patch2, and close this issue, while opening
another JIRA for the java.lang.ThreadGroup.java issue. The goal is to
solve this java.lang.ThreadTest failure finally.
Thanks,
xiaofeng
--
http://xiao-feng.blogspot.com
> [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.
[jira] Updated: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Mikhail Markov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mikhail Markov updated HARMONY-3883:
------------------------------------
Attachment: H-3883_add_alt2.patch
H-3883_add_alt1.patch
I agree with your concerns.
Actually I was thinking about this gap while preparing the previous patch but thought that it's so smal that atomicity will never be broken.
I have 2 alternatives of fixing this:
1) The simplest and most obvious one: just add try/catch block to the toArray() methods (see H-3883_add_alt1.patch)
2) Avoid catching exceptions by adding special private method ensuring operation atomicity but not throwing NoSuchElementException (see H-3883_add_alt2.patch)
Which one do we prefer? (I personally like the 2nd one).
> [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.
[jira] Commented: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Xiao-Feng Li (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498623 ]
Xiao-Feng Li commented on HARMONY-3883:
---------------------------------------
H-3883_add_final.patch is committed.
> [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, H-3883_add_final.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.
[jira] Commented: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Xiao-Feng Li (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498142 ]
Xiao-Feng Li commented on HARMONY-3883:
---------------------------------------
To correct the behavior, probably we can loop in the addNext() to find
the first non-NULL value. When a NULL is met, just loop back to get
next entry. Something like:
public boolean addNext() {
if (expectedModCount == modCount) {
while (hasNext()) {
currentEntry = nextEntry;
nextEntry = currentEntry.next;
R result = type.get(currentEntry);
// free the key
if(result == null) continue; // <--- add this line
nextKey = null;
return TRUE;
}
return FALSE
}
throw new ConcurrentModificationException();
}
Note the code is only for example of the idea, not tested.
I am not sure if we want to modify Next() similarly.
Thanks,
xiaofeng
--
http://xiao-feng.blogspot.com
> [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.
[jira] Updated: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Mikhail Markov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mikhail Markov updated HARMONY-3883:
------------------------------------
Attachment: H-3883_add_final.patch
Thanks, Xiao-Feng!
Here is the patch, which looks very simple removing inconsistency between hasNext() & next() methods + regression test.
> [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, H-3883_add_final.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.
[jira] Reopened: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Xiao-Feng Li (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Xiao-Feng Li reopened HARMONY-3883:
-----------------------------------
reopened and last patch reverted
> [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.
[jira] Commented: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Mikhail Markov (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Closed: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Xiao-Feng Li (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Xiao-Feng Li closed HARMONY-3883.
---------------------------------
Resolution: Fixed
H-3883_add_alt2.patch is committed.
> [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.
[jira] Commented: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Mikhail Markov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498255 ]
Mikhail Markov commented on HARMONY-3883:
-----------------------------------------
Hmm... null entries are allowed in the WeakHashMap. See the following valid code:
import java.util.WeakHashMap;
public class Test {
public static void main(String[] args) throws Exception {
WeakHashMap map = new WeakHashMap();
map.put("1", null);
map.put(null, null);
map.put("2", null);
Object[] keys = map.keySet().toArray();
for (int i = 0; i < keys.length; ++i) {
System.out.println("key[" + i + "] = " + keys[i]);
}
}
}
So, the proposed idea does not work as it will remove valid null entry ...
> [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.
[jira] Commented: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Xiao-Feng Li (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12496577 ]
Xiao-Feng Li commented on HARMONY-3883:
---------------------------------------
I've commited the patch.
> [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, 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.
[jira] Commented: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Li-Gang Wang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12496598 ]
Li-Gang Wang commented on HARMONY-3883:
---------------------------------------
Mikhail, thanks for your attention and productive solution.
I think the solution is basically right. But this can not totally eliminate the failure possibility, although it reduces the possibility to a very low degree. The reason is:
+ for (Iterator<K> iter = iterator(); iter.hasNext();) {
+ coll.add(iter.next());
+ }
In the 'for' loop, when it comes to the last element, a situation might happen: iter.hasNext() returns true, so the execution flow runs to coll.add(iter.next()). At this point, GC happens and the last element's key is cleared. Then in iter.next(), iter.hasNext() will be called again. This time it returns false, hence iter.next() will also throw a NoSuchElementException.
Mihail, what do you think of this?
> [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, 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.
[jira] Commented: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Li-Gang Wang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12496483 ]
Li-Gang Wang commented on HARMONY-3883:
---------------------------------------
Running the modified WeakHashmapTest, the problem can appear whether using GCv5 or GCv4.1.
> [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: 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.
[jira] Updated: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Mikhail Markov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mikhail Markov updated HARMONY-3883:
------------------------------------
Attachment: H-3883.patch
Thanks, Li-Gang - excellent catch!
Here is the fix for the issue - added toArray() methods to the inner Set returned by keySet() method.
> [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, 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.
[jira] Commented: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Mikhail Markov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498627 ]
Mikhail Markov commented on HARMONY-3883:
-----------------------------------------
Xiao-Feng, thanks - integrated as expected!
IMO this time the issue could be really closed :-).
> [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, H-3883_add_final.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.
[jira] Updated: (HARMONY-3883)
[classlib]WeakHashMap.keySet().toArray() intermittently throws
NoSuchElementException in java.lang.ThreadTest of DRLVM kernel test
Posted by "Li-Gang Wang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3883?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Li-Gang Wang updated HARMONY-3883:
----------------------------------
Attachment: WeakHashmapTest.java
I modified the WeakHashmapTest.java in DRLVM smoke test so that it can easily make this problem appear. Just replace the original one in $drlvm/vm/tests/smoke/stress/WeakHashmapTest.java with the attachment and try to run it individually.
> [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: 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.