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) <[EMAIL PROTECTED]> 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
