Costin Manolache wrote:
You're probably right that addThread() doesn't need to be public - butIt does when you're trying to refactor a class. It forces you to ask whether the interface really is used, or can you rearrange like you want to. Usually I grep the sources to determine that, but when you're using reflection/introspection (e.g., JMX), then there's no telling. You have to take an educated guess and hope that you don't break things.
I don't think it hurts too much either.
It also prevents the byte code compiler from doing complete, inlined optimization when a member is non-private.
What classes are you referring to specifically? StandardManager & WebappLoader?Also: MonitorRunnable, now that it has a start() method (for JMX, I presume) has no restriction on running more than one thread. This undercuts the meaning of "interval", and makes stop() poorly defined.
IMO MonitorRunnable, the session expiration thread ( one per ctx ) and the reloading thread ( one per ctx again )
should all go away andI'd hate to think what would happen if someone stopped the timer service. Also, using the event-driven approach to trigger maintenance activities in foreground threads would be quirky coding, and may not even be appropriate in classes that don't always have executing code.
be replaced by a single mechanism. Maybe the JMX timer ?
So I'd opt for java.util.Timer. It requires J2SE 1.3+...is that a problem? Here's what it looks like:
* Replace MonitorRunnable with java.util.Timer.
* The Timer thread is always daemonized; I couldn't understand why
MonitorRunnable would conditionally do so based on ThreadPool.
* Unlike the start/stop() MonitorRunnable i/f--which I assumed was
for activating the new wait() interval--the Timer can never be
stopped w/o shutting down the ThreadPool.
Index: ThreadPool.java
===================================================================
RCS file:
/home/cvspublic/jakarta-tomcat-connectors/util/java/org/apache/tomcat/util/threads/ThreadPool.java,v
retrieving revision 1.6
diff -u -r1.6 ThreadPool.java
--- ThreadPool.java 24 Dec 2002 17:02:54 -0000 1.6
+++ ThreadPool.java 28 Dec 2002 08:29:12 -0000
@@ -90,6 +90,34 @@
*/
protected MonitorRunnable monitor;
+ /**
+ * Timer for scheduling maintenance runs.
+ * A lock on this variable should be considered to apply to
+ * {@link #maintenanceInterval} as well.
+ */
+ private Timer maintenanceTimer = null;
+
+ /** Milliseconds between maintenance runs */
+
+ private long maintenanceInterval = WORK_WAIT_TIMEOUT;
+
+ /**
+ * Maintenance task to remove idle <code>ControlRunnable</code>s
+ */
+ private class MaintenanceTask extends TimerTask
+ {
+ public void run()
+ {
+ try {
+ checkSpareControllers();
+ }
+ catch (Throwable t) {
+ log.error( "maintenance task exception (continuing task anyway):" + t
+);
+ // don't cancel(), keep trying
+ }
+ }
+ }
+
/*
* Max number of threads that you can open in the pool.
@@ -158,12 +186,39 @@
openThreads(minSpareThreads);
monitor = new MonitorRunnable(this);
+ startMaintenanceTimer();
+ }
+
+
+ /**
+ * Sets the time interval between maintenance runs.
+ * Silently enforces a minimum (currently 10 milliseconds).
+ * Also re-creates the maintenance thread.
+ */
+ public void setMaintenanceInterval( long milliseconds )
+ {
+ if (milliseconds < 10) milliseconds = 10;
+
+ synchronized (maintenanceTimer) {
+ maintenanceInterval = milliseconds;
+ startMaintenanceTimer();
+ }
}
- public MonitorRunnable getMonitor() {
- return monitor;
+
+ /** (Re)start the maintenance timer. */
+
+ private void startMaintenanceTimer()
+ {
+ synchronized (maintenanceTimer) {
+ if (maintenanceTimer != null) maintenanceTimer.cancel();
+ maintenanceTimer = new Timer( true );
+ maintenanceTimer.schedule( new MaintenanceTask(), 0,
+ maintenanceInterval );
+ }
}
+
public void setMaxThreads(int maxThreads) {
this.maxThreads = maxThreads;
}
@@ -319,8 +374,14 @@
public synchronized void shutdown() {
if(!stopThePool) {
stopThePool = true;
- monitor.terminate();
- monitor = null;
+
+ synchronized (maintenanceTimer) {
+ if (maintenanceTimer != null) {
+ maintenanceTimer.cancel();
+ maintenanceTimer = null;
+ }
+ }
+
for(int i = 0 ; i < (currentThreadCount - currentThreadsBusy - 1) ; i++) {
try {
pool[i].terminate();
@@ -443,66 +504,6 @@
//loghelper.flush();
}
- /**
- * Periodically execute an action - cleanup in this case
- */
- public static class MonitorRunnable implements Runnable {
- ThreadPool p;
- Thread t;
- int interval=WORK_WAIT_TIMEOUT;
- boolean shouldTerminate;
-
- MonitorRunnable(ThreadPool p) {
- this.p=p;
- this.start();
- }
-
- public void start() {
- shouldTerminate = false;
- t = new Thread(this);
- t.setDaemon(p.getDaemon() );
- t.setName( "MonitorRunnable" );
- t.start();
- }
-
- public void setInterval(int i ) {
- this.interval=i;
- }
-
- public void run() {
- while(true) {
- try {
- // Sleep for a while.
- synchronized(this) {
- this.wait(WORK_WAIT_TIMEOUT);
- }
-
- // Check if should terminate.
- // termination happens when the pool is shutting down.
- if(shouldTerminate) {
- break;
- }
-
- // Harvest idle threads.
- p.checkSpareControllers();
-
- } catch(Throwable t) {
- ThreadPool.log.error("Unexpected exception", t);
- }
- }
- }
-
- public void stop() {
- this.terminate();
- }
-
- /** Stop the monitor
- */
- public synchronized void terminate() {
- shouldTerminate = true;
- this.notify();
- }
- }
/**
* A Thread object that executes various actions ( ThreadPoolRunnable )
-- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
