You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by wj...@apache.org on 2007/10/13 03:16:42 UTC
svn commit: r584356 - in /harmony/enhanced/drlvm/trunk/vm:
tests/kernel/java/lang/ThreadGroupTest.java
vmcore/src/kernel_classes/javasrc/java/lang/Thread.java
vmcore/src/kernel_classes/javasrc/java/lang/ThreadGroup.java
Author: wjwashburn
Date: Fri Oct 12 18:16:08 2007
New Revision: 584356
URL: http://svn.apache.org/viewvc?rev=584356&view=rev
Log:
Harmony-4766: this patch fixes problems with ThreadGroup.destroy() being
called on non-started threads
Modified:
harmony/enhanced/drlvm/trunk/vm/tests/kernel/java/lang/ThreadGroupTest.java
harmony/enhanced/drlvm/trunk/vm/vmcore/src/kernel_classes/javasrc/java/lang/Thread.java
harmony/enhanced/drlvm/trunk/vm/vmcore/src/kernel_classes/javasrc/java/lang/ThreadGroup.java
Modified: harmony/enhanced/drlvm/trunk/vm/tests/kernel/java/lang/ThreadGroupTest.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/tests/kernel/java/lang/ThreadGroupTest.java?rev=584356&r1=584355&r2=584356&view=diff
==============================================================================
--- harmony/enhanced/drlvm/trunk/vm/tests/kernel/java/lang/ThreadGroupTest.java (original)
+++ harmony/enhanced/drlvm/trunk/vm/tests/kernel/java/lang/ThreadGroupTest.java Fri Oct 12 18:16:08 2007
@@ -273,26 +273,33 @@
*/
public void testActiveGroupCount_DestroyNonEmptySubgroup() {
ThreadGroup tg1 = new ThreadGroup("group 1");
- new ThreadGroup(tg1, "group 11");
+ ThreadGroup tg11 = new ThreadGroup(tg1, "group 11");
ThreadGroup tg12 = new ThreadGroup(tg1, "group 12");
- new ThreadGroup(tg1, "group 13");
- new ThreadGroup(tg12, "group 121");
- new ThreadGroup(tg12, "group 122");
+ ThreadGroup tg13 = new ThreadGroup(tg1, "group 13");
+ ThreadGroup tg121 = new ThreadGroup(tg12, "group 121");
+ ThreadGroup tg122 = new ThreadGroup(tg12, "group 122");
ThreadGroup tg123 = new ThreadGroup(tg12, "group 123");
- new ThreadTest.ThreadRunning(tg123, "thread 1231").start();
- new ThreadTest.ThreadRunning(tg123, "thread 1232").start();
- // tg1 and its non-empty subgroups tg123 and tg12
- // should not be destroyed.
- // All empty subgroups shoud be destroyed.
+ new ThreadTest.ThreadRunning(tg122, "thread 1221").start();
+ new ThreadTest.ThreadRunning(tg122, "thread 1222").start();
+ // Non-empty subgroup tg122 should not be destroyed.
// IllegalThreadStateException should be thrown.
+ // Groups residing on the right from tg123 in the groups tree
+ // should not be destroyed as well.
try {
tg1.destroy();
fail("IllegalThreadStateException has not been thrown when " +
"destroying non-empty subgroup");
} catch (IllegalThreadStateException e) {
}
- assertEquals("wrong group count in tg1", 2, tg1.activeGroupCount());
- assertEquals("wrong group count in tg12", 1, tg12.activeGroupCount());
+ assertEquals("wrong group count in tg1", 0, tg1.activeGroupCount());
+ assertEquals("wrong group count in tg12", 0, tg12.activeGroupCount());
+ assertTrue("tg1 is not destroyed", tg1.isDestroyed());
+ assertTrue("tg11 is not destroyed", tg11.isDestroyed());
+ assertTrue("tg12 is not destroyed", tg12.isDestroyed());
+ assertTrue("tg13 is destroyed", !tg13.isDestroyed());
+ assertTrue("tg121 is not destroyed", tg121.isDestroyed());
+ assertTrue("tg122 is destroyed", !tg122.isDestroyed());
+ assertTrue("tg123 is destroyed", !tg123.isDestroyed());
}
/**
Modified: harmony/enhanced/drlvm/trunk/vm/vmcore/src/kernel_classes/javasrc/java/lang/Thread.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/vmcore/src/kernel_classes/javasrc/java/lang/Thread.java?rev=584356&r1=584355&r2=584356&view=diff
==============================================================================
--- harmony/enhanced/drlvm/trunk/vm/vmcore/src/kernel_classes/javasrc/java/lang/Thread.java (original)
+++ harmony/enhanced/drlvm/trunk/vm/vmcore/src/kernel_classes/javasrc/java/lang/Thread.java Fri Oct 12 18:16:08 2007
@@ -274,6 +274,9 @@
if (threadGroup == null) {
threadGroup = currentThread.group;
}
+
+ threadGroup.checkGroup();
+
this.group = threadGroup;
this.daemon = currentThread.daemon;
this.contextClassLoader = currentThread.contextClassLoader;
@@ -301,8 +304,6 @@
SecurityUtils.putContext(this, AccessController.getContext());
checkAccess();
- // adding the thread to the thread group should be the last action
- threadGroup.add(this);
}
/**
@@ -747,6 +748,9 @@
throw new IllegalThreadStateException(
"This thread was already started!");
}
+
+ // adding the thread to the thread group
+ group.add(this);
if (VMThreadManager.start(this, stackSize, daemon, priority) != 0) {
Modified: harmony/enhanced/drlvm/trunk/vm/vmcore/src/kernel_classes/javasrc/java/lang/ThreadGroup.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/vmcore/src/kernel_classes/javasrc/java/lang/ThreadGroup.java?rev=584356&r1=584355&r2=584356&view=diff
==============================================================================
--- harmony/enhanced/drlvm/trunk/vm/vmcore/src/kernel_classes/javasrc/java/lang/ThreadGroup.java (original)
+++ harmony/enhanced/drlvm/trunk/vm/vmcore/src/kernel_classes/javasrc/java/lang/ThreadGroup.java Fri Oct 12 18:16:08 2007
@@ -25,8 +25,6 @@
import java.util.ArrayList;
import java.util.LinkedList;
import java.util.Iterator;
-import java.util.WeakHashMap;
-import java.util.ConcurrentModificationException;
/**
* @com.intel.drl.spec_ref
@@ -83,7 +81,7 @@
/**
* All threads in the group.
*/
- private WeakHashMap<Thread, ThreadGroup> threads = new WeakHashMap<Thread, ThreadGroup>();
+ private LinkedList<Thread> threads = new LinkedList<Thread>();
/**
* @com.intel.drl.spec_ref
@@ -123,24 +121,24 @@
*/
public int activeCount() {
int count = 0;
- List groupsListCopy = null; // a copy of subgroups list
- Object[] threadsCopy = null; // a copy of threads list
+ List groupsCopy = null; // a copy of subgroups list
+ List threadsCopy = null; // a copy of threads list
synchronized (lock) {
if (destroyed) {
return 0;
}
- threadsCopy = copyThreads();
- groupsListCopy = (List)groups.clone();
+ threadsCopy = (List)threads.clone();
+ groupsCopy = (List)groups.clone();
}
- for (int i = 0; i < threadsCopy.length; i++) {
- if (((Thread) threadsCopy[i]).isAlive()) {
+ for (Object thread : threadsCopy) {
+ if (((Thread)thread).isAlive()) {
count++;
}
}
- for (Iterator it = groupsListCopy.iterator(); it.hasNext();) {
- count += ((ThreadGroup)it.next()).activeCount();
+ for (Object group : groupsCopy) {
+ count += ((ThreadGroup)group).activeCount();
}
return count;
}
@@ -150,16 +148,16 @@
*/
public int activeGroupCount() {
int count;
- List groupsListCopy = null; // a copy of subgroups list
+ List groupsCopy = null; // a copy of subgroups list
synchronized (lock) {
if (destroyed) {
return 0;
}
count = groups.size();
- groupsListCopy = (List)groups.clone();
+ groupsCopy = (List)groups.clone();
}
- for (Iterator it = groupsListCopy.iterator(); it.hasNext();) {
- count += ((ThreadGroup)it.next()).activeGroupCount();
+ for (Object group : (List)groupsCopy) {
+ count += ((ThreadGroup)group).activeGroupCount();
}
return count;
}
@@ -193,14 +191,7 @@
throw new IllegalThreadStateException(
"The thread group " + name + " is already destroyed!");
}
- if (!nonsecureDestroy()) {
- throw new IllegalThreadStateException("The thread group " + name +
- " is not empty");
- } else {
- if (parent != null) {
- parent.remove(this);
- }
- }
+ nonsecureDestroy();
}
}
@@ -400,7 +391,19 @@
throw new IllegalThreadStateException(
"The thread group is already destroyed!");
}
- threads.put(thread, this);
+ threads.add(thread);
+ }
+ }
+
+ /**
+ * Checks that group is not destroyed
+ */
+ void checkGroup() {
+ synchronized (lock) {
+ if (destroyed) {
+ throw new IllegalThreadStateException(
+ "The thread group is already destroyed!");
+ }
}
}
@@ -438,21 +441,6 @@
}
/**
- * Copies this ThreadGroup's threads to an array which is used in iteration
- * over threads because iteration over a WeakHashMap object can lead to
- * ConcurrentModificationException.
- */
- private Object[] copyThreads() {
- while (true) {
- try {
- // toArray() can throw ConcurrentModificationException
- return threads.keySet().toArray();
- } catch (ConcurrentModificationException e) {
- }
- }
- }
-
- /**
* Used by GetThreadGroupChildren() jvmti function.
* @return Object[] array of 2 elements: first - Object[] array of active
* child threads; second - Object[] array of child groups.
@@ -467,7 +455,7 @@
return new Object[] {null, null};
}
- for (Thread thread : threads.keySet()) {
+ for (Thread thread : threads) {
threadsCopy.add(thread);
}
@@ -503,27 +491,27 @@
* done
*/
private int enumerate(Thread[] list, int offset, boolean recurse) {
- List groupsListCopy = null; // a copy of subgroups list
- Object[] threadsCopy = null; // a copy of threads list
+ List groupsCopy = null; // a copy of subgroups list
+ List threadsCopy = null; // a copy of threads list
synchronized (lock) {
if (destroyed) {
return offset;
}
- threadsCopy = copyThreads();
+ threadsCopy = (List)threads.clone();
if (recurse) {
- groupsListCopy = (List)groups.clone();
+ groupsCopy = (List)groups.clone();
}
}
- for (int i = 0; i < threadsCopy.length; i++) {
- if (((Thread) threadsCopy[i]).isAlive()) {
- list[offset++] = (Thread) threadsCopy[i];
+ for (Object thread : threadsCopy) {
+ if (((Thread)thread).isAlive()) {
+ list[offset++] = ((Thread)thread);
if (offset == list.length) {
return offset;
}
}
}
if (recurse) {
- for (Iterator it = groupsListCopy.iterator(); offset < list.length
+ for (Iterator it = groupsCopy.iterator(); offset < list.length
&& it.hasNext();) {
offset = ((ThreadGroup)it.next()).enumerate(list, offset, true);
}
@@ -551,8 +539,8 @@
}
int firstGroupIdx = offset;
synchronized (lock) {
- for (Iterator it = groups.iterator(); it.hasNext();) {
- list[offset++] = (ThreadGroup)it.next();
+ for (Object group : groups) {
+ list[offset++] = (ThreadGroup)group;
if (offset == list.length) {
return offset;
}
@@ -575,17 +563,17 @@
private void list(String prefix) {
System.out.println(prefix + toString());
prefix += LISTING_INDENT;
- List groupsListCopy = null; // a copy of subgroups list
- Object[] threadsCopy = null; // a copy of threads list
+ List groupsCopy = null; // a copy of subgroups list
+ List threadsCopy = null; // a copy of threads list
synchronized (lock) {
- threadsCopy = copyThreads();
- groupsListCopy = (List)groups.clone();
+ threadsCopy = (List)threads.clone();
+ groupsCopy = (List)groups.clone();
}
- for (int i = 0; i < threadsCopy.length; i++) {
- System.out.println(prefix + (Thread) threadsCopy[i]);
+ for (Object thread : threadsCopy) {
+ System.out.println(prefix + (Thread)thread);
}
- for (Iterator it = groupsListCopy.iterator(); it.hasNext();) {
- ((ThreadGroup)it.next()).list(prefix);
+ for (Object group : groupsCopy) {
+ ((ThreadGroup)group).list(prefix);
}
}
@@ -597,24 +585,26 @@
* will be thrown.
* @return false if this ThreadGroup is not empty
*/
- private boolean nonsecureDestroy() {
- boolean thisGroupIsEmpty = true;
+ private void nonsecureDestroy() {
+
+ List groupsCopy = null;
+
synchronized (lock) {
- if (!threads.isEmpty()) {
- return false;
- }
- for (Iterator<ThreadGroup> it = groups.iterator(); it.hasNext();) {
- if (it.next().nonsecureDestroy()) {
- it.remove();
- } else {
- thisGroupIsEmpty = false;
- }
- }
- if (groups.isEmpty()) {
- destroyed = true;
+ if (threads.size() > 0) {
+ throw new IllegalThreadStateException("The thread group " + name +
+ " is not empty");
}
+ destroyed = true;
+ groupsCopy = (List)groups.clone();
+ }
+
+ if (parent != null) {
+ parent.remove(this);
+ }
+
+ for (Object group : groupsCopy) {
+ ((ThreadGroup)group).nonsecureDestroy();
}
- return thisGroupIsEmpty;
}
/**
@@ -623,12 +613,11 @@
*/
private void nonsecureInterrupt() {
synchronized (lock) {
- Object[] threadsCopy = copyThreads(); // a copy of threads list
- for (int i = 0; i < threadsCopy.length; i++) {
- ((Thread) threadsCopy[i]).interrupt();
+ for (Object thread : threads) {
+ ((Thread)thread).interrupt();
}
- for (Iterator it = groups.iterator(); it.hasNext();) {
- ((ThreadGroup)it.next()).nonsecureInterrupt();
+ for (Object group : groups) {
+ ((ThreadGroup)group).nonsecureInterrupt();
}
}
}
@@ -639,12 +628,11 @@
*/
private void nonsecureResume() {
synchronized (lock) {
- Object[] threadsCopy = copyThreads();
- for (int i = 0; i < threadsCopy.length; i++) {
- ((Thread) threadsCopy[i]).resume();
+ for (Object thread : threads) {
+ ((Thread)thread).resume();
}
- for (Iterator it = groups.iterator(); it.hasNext();) {
- ((ThreadGroup)it.next()).nonsecureResume();
+ for (Object group : groups) {
+ ((ThreadGroup)group).nonsecureResume();
}
}
}
@@ -657,8 +645,8 @@
synchronized (lock) {
this.maxPriority = priority;
- for (Iterator it = groups.iterator(); it.hasNext();) {
- ((ThreadGroup)it.next()).nonsecureSetMaxPriority(priority);
+ for (Object group : groups) {
+ ((ThreadGroup)group).nonsecureSetMaxPriority(priority);
}
}
}
@@ -669,12 +657,11 @@
*/
private void nonsecureStop() {
synchronized (lock) {
- Object[] threadsCopy = copyThreads();
- for (int i = 0; i < threadsCopy.length; i++) {
- ((Thread) threadsCopy[i]).stop();
+ for (Object thread : threads) {
+ ((Thread)thread).stop();
}
- for (Iterator it = groups.iterator(); it.hasNext();) {
- ((ThreadGroup)it.next()).nonsecureStop();
+ for (Object group : groups) {
+ ((ThreadGroup)group).nonsecureStop();
}
}
}
@@ -685,12 +672,11 @@
*/
private void nonsecureSuspend() {
synchronized (lock) {
- Object[] threadsCopy = copyThreads(); // a copy of threads list
- for (int i = 0; i < threadsCopy.length; i++) {
- ((Thread) threadsCopy[i]).suspend();
+ for (Object thread : threads) {
+ ((Thread)thread).suspend();
}
- for (Iterator it = groups.iterator(); it.hasNext();) {
- ((ThreadGroup)it.next()).nonsecureSuspend();
+ for (Object group : groups) {
+ ((ThreadGroup)group).nonsecureSuspend();
}
}
}