harmony-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wjwashb...@apache.org
Subject 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
Date Sat, 13 Oct 2007 01:16:42 GMT
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();
             }
         }
     }



Mime
View raw message