Author: markt
Date: Thu Sep 30 17:56:42 2010
New Revision: 1003183
URL: http://svn.apache.org/viewvc?rev=1003183&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49730
Race condition in executor can lead to long delays in request processing
Modified:
tomcat/trunk/java/org/apache/tomcat/util/threads/TaskQueue.java
tomcat/trunk/java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/tomcat/util/threads/TaskQueue.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/threads/TaskQueue.java?rev=1003183&r1=1003182&r2=1003183&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/threads/TaskQueue.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/threads/TaskQueue.java Thu Sep 30
17:56:42 2010
@@ -19,8 +19,8 @@ package org.apache.tomcat.util.threads;
import java.util.Collection;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.RejectedExecutionException;
-import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
+
/**
* As task queue specifically designed to run with a thread pool executor.
* The task queue is optimised to properly utilize threads within
@@ -65,7 +65,7 @@ public class TaskQueue extends LinkedBlo
//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
- if (parent.getActiveCount()<(parent.getPoolSize())) return
super.offer(o);
+ if (parent.getSubmittedCount()<(parent.getPoolSize())) 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 we reached here, we need to add it to the queue
Modified:
tomcat/trunk/java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java?rev=1003183&r1=1003182&r2=1003183&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java
(original)
+++ tomcat/trunk/java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java
Thu Sep 30 17:56:42 2010
@@ -24,7 +24,7 @@ import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
/**
* Same as a java.util.concurrent.ThreadPoolExecutor but implements a much
more efficient
- * getActiveCount method, to be used to properly handle the work queue
+ * {...@link #getSubmittedCount()} method, to be used to properly handle the
work queue.
* If a RejectedExecutionHandler is not specified a default one will be
configured
* and that one will always throw a RejectedExecutionException
* @author fhanik
@@ -32,7 +32,13 @@ import java.util.concurrent.atomic.Atomi
*/
public class ThreadPoolExecutor extends
java.util.concurrent.ThreadPoolExecutor {
- private final AtomicInteger activeCount = new AtomicInteger(0);
+ /**
+ * The number of tasks submitted but not yet finished. This includes tasks
+ * in the queue and tasks that have been handed to a worker thread but the
+ * latter did not start executing the task yet.
+ * This number is always greater or equal to {...@link #getActiveCount()}.
+ */
+ private final AtomicInteger submittedCount = new AtomicInteger(0);
public ThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long
keepAliveTime, TimeUnit unit, BlockingQueue<Runnable> workQueue,
RejectedExecutionHandler handler) {
super(corePoolSize, maximumPoolSize, keepAliveTime, unit, workQueue,
handler);
@@ -53,17 +59,11 @@ public class ThreadPoolExecutor extends
@Override
protected void afterExecute(Runnable r, Throwable t) {
- activeCount.decrementAndGet();
- }
-
- @Override
- protected void beforeExecute(Thread t, Runnable r) {
- activeCount.incrementAndGet();
+ submittedCount.decrementAndGet();
}
- @Override
- public int getActiveCount() {
- return activeCount.get();
+ public int getSubmittedCount() {
+ return submittedCount.get();
}
/**
@@ -88,6 +88,7 @@ public class ThreadPoolExecutor extends
* @throws NullPointerException if command or unit is null
*/
public void execute(Runnable command, long timeout, TimeUnit unit) {
+ submittedCount.incrementAndGet();
try {
super.execute(command);
} catch (RejectedExecutionException rx) {
@@ -95,13 +96,16 @@ public class ThreadPoolExecutor extends
final TaskQueue queue = (TaskQueue)super.getQueue();
try {
if (!queue.force(command, timeout, unit)) {
+ submittedCount.decrementAndGet();
throw new RejectedExecutionException("Queue capacity
is full.");
}
} catch (InterruptedException x) {
+ submittedCount.decrementAndGet();
Thread.interrupted();
throw new RejectedExecutionException(x);
}
} else {
+ submittedCount.decrementAndGet();
throw rx;
}
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1003183&r1=1003182&r2=1003183&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Thu Sep 30 17:56:42 2010
@@ -87,6 +87,11 @@
managing the PID file and Tomcat does not have write access. (markt)
</fix>
<fix>
+ <bug>49730</bug>: Fix a race condition in StandardThreadExector that
can
+ cause requests to experience large delays. Patch provided by Sylvain
+ Laurent. (markt)
+ </fix>
+ <fix>
<bug>49749</bug>: Single sign on cookies should have httpOnly flag set
using same rules as session cookies. (markt)
</fix>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
