I wrote:
> I'm leaning towards "leaking" daemon thread groups in the case of
> unstarted threads that have been GCed.
I've modified my patch to use a PhantomReference to do the cleanup. I
still think we shouldn't bother to do the cleanup, but I will note that
using the PhantomReference probably isn't as expensive as I originally
thought, because it can be thrown away as soon as the thread is started.
Regards,
Jeroen
Index: java/lang/Thread.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/lang/Thread.java,v
retrieving revision 1.10
diff -u -r1.10 Thread.java
--- java/lang/Thread.java 6 Dec 2004 20:43:13 -0000 1.10
+++ java/lang/Thread.java 12 Dec 2004 14:21:44 -0000
@@ -38,6 +38,8 @@
package java.lang;
+import java.lang.ref.PhantomReference;
+import java.lang.ref.ReferenceQueue;
/* Written using "Java Class Libraries", 2nd edition, ISBN 0-201-31002-3
* "The Java Language Specification", ISBN 0-201-63451-1
@@ -88,6 +90,28 @@
*/
public class Thread implements Runnable
{
+ static class ThreadCleanup extends PhantomReference
+ {
+ private static final ReferenceQueue queue = new ReferenceQueue();
+ private ThreadGroup group;
+
+ ThreadCleanup(Thread thread)
+ {
+ super(thread, queue);
+ this.group = thread.getThreadGroup();
+ runCleanup();
+ }
+
+ static void runCleanup()
+ {
+ ThreadCleanup cleaner;
+ while ((cleaner = (ThreadCleanup)queue.poll()) != null)
+ {
+ cleaner.group.removeInactiveThread();
+ }
+ }
+ }
+
/** The minimum priority for a Thread. */
public static final int MIN_PRIORITY = 1;
@@ -97,6 +121,9 @@
/** The maximum priority for a Thread. */
public static final int MAX_PRIORITY = 10;
+ /** The clean up object. */
+ private ThreadCleanup cleaner;
+
/** The underlying VM thread, only set when the thread is actually running.
*/
volatile VMThread vmThread;
@@ -339,7 +366,10 @@
daemon = current.daemon;
contextClassLoader = current.contextClassLoader;
- group.addThread(this);
+ if (VMThread.CLEANUP_NONSTARTED_THREADS)
+ cleaner = new ThreadCleanup(this);
+
+ group.addInactiveThread();
InheritableThreadLocal.newChildThread(this);
}
@@ -823,7 +853,24 @@
if (vmThread != null || group == null)
throw new IllegalThreadStateException();
- VMThread.create(this, stacksize);
+ boolean ok = false;
+ try
+ {
+ if (VMThread.CLEANUP_NONSTARTED_THREADS);
+ {
+ cleaner.clear();
+ cleaner = null;
+ }
+ group.addThread(this);
+ group.removeInactiveThread();
+ VMThread.create(this, stacksize);
+ ok = true;
+ }
+ finally
+ {
+ if (!ok)
+ die();
+ }
}
/**
Index: java/lang/ThreadGroup.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/lang/ThreadGroup.java,v
retrieving revision 1.17
diff -u -r1.17 ThreadGroup.java
--- java/lang/ThreadGroup.java 6 Dec 2004 20:43:13 -0000 1.17
+++ java/lang/ThreadGroup.java 12 Dec 2004 14:21:45 -0000
@@ -70,9 +70,12 @@
/** The group name, non-null. */
final String name;
- /** The threads in the group. */
+ /** The active threads in the group. */
private final Vector threads = new Vector();
+ /** The number of inactive threads in the group */
+ private int inactiveThreads;
+
/** Child thread groups, or null when this group is destroyed. */
private Vector groups = new Vector();
@@ -188,6 +191,9 @@
*/
public synchronized boolean isDestroyed()
{
+ if (VMThread.CLEANUP_NONSTARTED_THREADS)
+ Thread.ThreadCleanup.runCleanup();
+
return groups == null;
}
@@ -695,7 +701,8 @@
}
/**
- * Add a thread to the group. Called by Thread constructors.
+ * Add a thread to the group. Called by the VM for Threads
+ * started in native code, or by Thread.start() for Java threads.
*
* @param t the thread to add, non-null
* @throws IllegalThreadStateException if the group is destroyed
@@ -708,6 +715,25 @@
}
/**
+ * Increment the of inactive thread count. Whenever a Thread
+ * object is created this count should be incremented so that
+ * we know that the group isn't empty.
+ */
+ final synchronized void addInactiveThread()
+ {
+ inactiveThreads++;
+ }
+
+ /**
+ * Decrement the inactive thread count.
+ */
+ final synchronized void removeInactiveThread()
+ {
+ inactiveThreads--;
+ checkForDaemonDeath();
+ }
+
+ /**
* Called by the VM to remove a thread that has died.
*
* @param t the thread to remove, non-null
@@ -719,14 +745,7 @@
return;
threads.remove(t);
t.group = null;
- // Daemon groups are automatically destroyed when all their threads die.
- if (daemon_flag && groups.size() == 0 && threads.size() == 0)
- {
- // We inline destroy to avoid the access check.
- groups = null;
- if (parent != null)
- parent.removeGroup(this);
- }
+ checkForDaemonDeath();
}
/**
@@ -737,8 +756,17 @@
final synchronized void removeGroup(ThreadGroup g)
{
groups.remove(g);
+ checkForDaemonDeath();
+ }
+
+ private void checkForDaemonDeath()
+ {
+ if (VMThread.CLEANUP_NONSTARTED_THREADS)
+ Thread.ThreadCleanup.runCleanup();
+
// Daemon groups are automatically destroyed when all their threads die.
- if (daemon_flag && groups.size() == 0 && threads.size() == 0)
+ if (daemon_flag && groups.size() == 0
+ && threads.size() == 0 && inactiveThreads == 0)
{
// We inline destroy to avoid the access check.
groups = null;
Index: vm/reference/java/lang/VMThread.java
===================================================================
RCS file: /cvsroot/classpath/classpath/vm/reference/java/lang/VMThread.java,v
retrieving revision 1.2
diff -u -r1.2 VMThread.java
--- vm/reference/java/lang/VMThread.java 28 Jun 2004 19:39:16 -0000
1.2
+++ vm/reference/java/lang/VMThread.java 12 Dec 2004 14:21:56 -0000
@@ -69,6 +69,8 @@
*/
final class VMThread
{
+ static final boolean CLEANUP_NONSTARTED_THREADS = true;
+
/**
* The Thread object that this VM state belongs to.
* Used in currentThread() and start().
_______________________________________________
Classpath mailing list
[EMAIL PROTECTED]
http://lists.gnu.org/mailman/listinfo/classpath
