[PATCH] Re: implement ThreadPool with GenericObjectPool?

[Michael]

 Costin Manolache wrote:

The main reason I'm cautious - this is a very critical piece of code.

I also value stability (and more than performance). I'm tired of backing out and waiting to upgrade TC releases. (4.1.16 works for me, BTW.)

the debugging feature (JMX support) you're adding.

JMX is not a debugging feature - it's monitoring/control. It would allow people to see how many threads are used, to eventually stop threads or change the parameters dynamically.

I know what JMX itself is--I'm going by the check-in comment:

I want to add JMX support and make calls to store the
"state" of the thread, i.e. what is the thread doing.
This would allow people to debug hunged threads and
allow more control.

Perhaps I misunderstood this to mean debugging of the TC code, rather than the servlet's.


In any case, ThreadPool will at least need this patch, since removeThread() is never called. I also can't imagine why addThread() and removeThread() would need to be more than "private", given their role. Other liberal access modifiers abound.

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.

Index: ThreadPool.java
===================================================================
RCS file: 
/home/cvspublic/jakarta-tomcat-connectors/util/java/org/apache/tomcat/util/threads/ThreadPool.java,v
retrieving revision 1.5
diff -u -r1.5 ThreadPool.java
--- ThreadPool.java     19 Dec 2002 05:45:42 -0000      1.5
+++ ThreadPool.java     24 Dec 2002 05:08:09 -0000
@@ -560,7 +560,7 @@
         }
 
         public void run() {
-
+           try {
             while(true) {
                 try {
                            /* Wait for work. */
@@ -626,6 +626,9 @@
                    p.log.error("Unexpected exception", ie);
                 }
             }
+           } finally {
+               p.removeThread( Thread.currentThread() );
+           }
         }
 
         /** Run a task



--
To unsubscribe, e-mail:   <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>