You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2010/11/04 15:25:26 UTC

svn commit: r1031001 - in /tomcat/tc6.0.x/trunk: STATUS.txt java/org/apache/catalina/core/StandardThreadExecutor.java webapps/docs/changelog.xml

Author: markt
Date: Thu Nov  4 14:25:26 2010
New Revision: 1031001

URL: http://svn.apache.org/viewvc?rev=1031001&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49730
Correct race condition in StandardThreadExecutor that can lead to long delays in processing requests. Patch provided by Sylvain Laurent.

Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/StandardThreadExecutor.java
    tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1031001&r1=1031000&r2=1031001&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Thu Nov  4 14:25:26 2010
@@ -102,13 +102,6 @@ PATCHES PROPOSED TO BACKPORT:
    but from debugging it looks that it is called by Tomcat code only
    (JspServlet).
 
-* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49730
-  Correct race condition in StandardThreadExecutor that can lead to long delays
-  in processing requests. Patch provided by Sylvain Laurent.
-  https://issues.apache.org/bugzilla/attachment.cgi?id=25865
-  +1: markt, rjung, funkman
-  -1:
-
 * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49860
   Add support for trailing headers.
   http://svn.apache.org/viewvc?rev=1003461&view=rev

Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/StandardThreadExecutor.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/StandardThreadExecutor.java?rev=1031001&r1=1031000&r2=1031001&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/StandardThreadExecutor.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/StandardThreadExecutor.java Thu Nov  4 14:25:26 2010
@@ -49,6 +49,11 @@ public class StandardThreadExecutor impl
     
     protected String name;
     
+    /**
+     * Number of tasks submitted and not yet completed.
+     */
+    protected AtomicInteger submittedTasksCount;
+    
     private LifecycleSupport lifecycle = new LifecycleSupport(this);
     // ---------------------------------------------- Constructors
     public StandardThreadExecutor() {
@@ -63,8 +68,17 @@ public class StandardThreadExecutor impl
         TaskQueue taskqueue = new TaskQueue();
         TaskThreadFactory tf = new TaskThreadFactory(namePrefix);
         lifecycle.fireLifecycleEvent(START_EVENT, null);
-        executor = new ThreadPoolExecutor(getMinSpareThreads(), getMaxThreads(), maxIdleTime, TimeUnit.MILLISECONDS,taskqueue, tf);
+        executor = new ThreadPoolExecutor(getMinSpareThreads(), getMaxThreads(), maxIdleTime, TimeUnit.MILLISECONDS,taskqueue, tf) {
+			@Override
+			protected void afterExecute(Runnable r, Throwable t) {
+				AtomicInteger atomic = submittedTasksCount;
+				if(atomic!=null) {
+					atomic.decrementAndGet();
+				}
+			}
+        };
         taskqueue.setParent( (ThreadPoolExecutor) executor);
+        submittedTasksCount = new AtomicInteger();
         lifecycle.fireLifecycleEvent(AFTER_START_EVENT, null);
     }
     
@@ -73,16 +87,21 @@ public class StandardThreadExecutor impl
         lifecycle.fireLifecycleEvent(STOP_EVENT, null);
         if ( executor != null ) executor.shutdown();
         executor = null;
+        submittedTasksCount = null;
         lifecycle.fireLifecycleEvent(AFTER_STOP_EVENT, null);
     }
     
     public void execute(Runnable command) {
         if ( executor != null ) {
+        	submittedTasksCount.incrementAndGet();
             try {
                 executor.execute(command);
             } catch (RejectedExecutionException rx) {
                 //there could have been contention around the queue
-                if ( !( (TaskQueue) executor.getQueue()).force(command) ) throw new RejectedExecutionException();
+                if ( !( (TaskQueue) executor.getQueue()).force(command) ) {
+                	submittedTasksCount.decrementAndGet();
+                	throw new RejectedExecutionException();
+                }
             }
         } else throw new IllegalStateException("StandardThreadPool not started.");
     }
@@ -234,13 +253,17 @@ public class StandardThreadExecutor impl
         public boolean offer(Runnable o) {
             //we can't do any checks
             if (parent==null) return super.offer(o);
+            int poolSize = parent.getPoolSize();
             //we are maxed out on threads, simply queue the object
             if (parent.getPoolSize() == parent.getMaximumPoolSize()) return super.offer(o);
             //we have idle threads, just add it to the queue
-            //this is an approximation, so it could use some tuning
-            if (parent.getActiveCount()<(parent.getPoolSize())) return super.offer(o);
+            //note that we don't use getActiveCount(), see BZ 49730
+			AtomicInteger submittedTasksCount = StandardThreadExecutor.this.submittedTasksCount;
+			if(submittedTasksCount!=null) {
+				if (submittedTasksCount.get()<=poolSize) return super.offer(o);
+			}
             //if we have less threads than maximum force creation of a new thread
-            if (parent.getPoolSize()<parent.getMaximumPoolSize()) return false;
+            if (poolSize<parent.getMaximumPoolSize()) return false;
             //if we reached here, we need to add it to the queue
             return super.offer(o);
         }

Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1031001&r1=1031000&r2=1031001&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Thu Nov  4 14:25:26 2010
@@ -195,6 +195,11 @@
         compressed rather than only setting it if it is compressed. (markt)
       </fix>
       <fix>
+        <bug>49730</bug>: Fix race condition in StandardThreadExecutor that can
+        lead to long delays in processing requests. Patch provided by Sylvain
+        Laurent. (markt)
+      </fix>
+      <fix>
         <bug>49972</bug>: Fix potential thread safe issue when formatting dates
         for use in HTTP headers. (markt)
       </fix>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org