David Holmes wrote:
> Jeroen writes:
> > I've attached my first stab at implementing this.
>
> Looks good to me. The only thing I'm a little concerned with is the
> finalize() method. I know it is the correct thing to do, to avoid a
> different form of "leak", but I dislike the idea of adding
> finalizers to core classes like Thread.
I didn't like it either and after thinking a little more about it I
dislike it even more, because subclasses that override finalize and
forget to call the base class (in my experience this is *highly* likely)
would cause the leak anyway.
Experimentation seems to indicate that Sun actually "leaks" unstarted
threads in daemon groups... Maybe we should do the same?
Oh and I realized that my patch wasn't quite correct. Attached is an
improved version.
Regards,
Jeroen
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 10 Dec 2004 13:46:58 -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();
@@ -695,7 +698,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 +712,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 +742,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 +753,14 @@
final synchronized void removeGroup(ThreadGroup g)
{
groups.remove(g);
+ checkForDaemonDeath();
+ }
+
+ private void checkForDaemonDeath()
+ {
// 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: 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 10 Dec 2004 13:46:58 -0000
@@ -339,7 +339,7 @@
daemon = current.daemon;
contextClassLoader = current.contextClassLoader;
- group.addThread(this);
+ group.addInactiveThread();
InheritableThreadLocal.newChildThread(this);
}
@@ -824,6 +824,9 @@
throw new IllegalThreadStateException();
VMThread.create(this, stacksize);
+
+ group.addThread(this);
+ group.removeInactiveThread();
}
/**
@@ -970,4 +973,14 @@
group.removeThread(this);
vmThread = null;
}
+
+ /**
+ * If we die before ever having been actived, we need to remove
+ * ourself from the ThreadGroup inactive count.
+ */
+ protected void finalize() throws Throwable
+ {
+ if (group != null)
+ group.removeInactiveThread();
+ }
}
_______________________________________________
Classpath mailing list
[EMAIL PROTECTED]
http://lists.gnu.org/mailman/listinfo/classpath
