This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/master by this push:
     new d3a5b1c  Refactor Servlet 3 async timeouts to become a more generic 
timeout
d3a5b1c is described below

commit d3a5b1cf5c60e6acea61286eb050e54f05d193ff
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Aug 7 18:42:44 2019 +0100

    Refactor Servlet 3 async timeouts to become a more generic timeout
    
    The timeout mechanism now represents any timeout that is triggered
    independently from the Socket read/write timeouts.
---
 java/org/apache/coyote/AbstractProcessor.java      |  9 +++++-
 java/org/apache/coyote/AbstractProtocol.java       | 34 +++++++++++++---------
 java/org/apache/coyote/Processor.java              | 14 +++++----
 .../http11/upgrade/UpgradeProcessorBase.java       | 10 ++++---
 4 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/java/org/apache/coyote/AbstractProcessor.java 
b/java/org/apache/coyote/AbstractProcessor.java
index d94a8e4..1affefa 100644
--- a/java/org/apache/coyote/AbstractProcessor.java
+++ b/java/org/apache/coyote/AbstractProcessor.java
@@ -39,7 +39,7 @@ import org.apache.tomcat.util.res.StringManager;
 
 /**
  * Provides functionality and attributes common to all supported protocols
- * (currently HTTP and AJP).
+ * (currently HTTP and AJP) for processing a single request/response.
  */
 public abstract class AbstractProcessor extends AbstractProcessorLight 
implements ActionHook {
 
@@ -628,6 +628,13 @@ public abstract class AbstractProcessor extends 
AbstractProcessorLight implement
     }
 
 
+    /**
+     * {@inheritDoc}
+     * <p>
+     * Sub-classes of this base class represent a single request/response pair.
+     * The timeout to be processed is, therefore, the Servlet asynchronous
+     * processing timeout.
+     */
     @Override
     public void timeoutAsync(long now) {
         if (now < 0) {
diff --git a/java/org/apache/coyote/AbstractProtocol.java 
b/java/org/apache/coyote/AbstractProtocol.java
index 174c899..d95d425 100644
--- a/java/org/apache/coyote/AbstractProtocol.java
+++ b/java/org/apache/coyote/AbstractProtocol.java
@@ -96,9 +96,9 @@ public abstract class AbstractProtocol<S> implements 
ProtocolHandler,
             Collections.newSetFromMap(new ConcurrentHashMap<Processor, 
Boolean>());
 
     /**
-     * Controller for the async timeout scheduling.
+     * Controller for the timeout scheduling.
      */
-    private ScheduledFuture<?> asyncTimeoutFuture = null;
+    private ScheduledFuture<?> timeoutFuture = null;
     private ScheduledFuture<?> monitorFuture;
 
     public AbstractProtocol(AbstractEndpoint<S,?> endpoint) {
@@ -595,17 +595,22 @@ public abstract class AbstractProtocol<S> implements 
ProtocolHandler,
     }
 
 
+    /**
+     * Note: The name of this method originated with the Servlet 3.0
+     * asynchronous processing but evolved over time to represent a timeout 
that
+     * is triggered independently of the socket read/write timeouts.
+     */
     protected void startAsyncTimeout() {
-        if (asyncTimeoutFuture == null || (asyncTimeoutFuture != null && 
asyncTimeoutFuture.isDone())) {
-            if (asyncTimeoutFuture != null && asyncTimeoutFuture.isDone()) {
+        if (timeoutFuture == null || (timeoutFuture != null && 
timeoutFuture.isDone())) {
+            if (timeoutFuture != null && timeoutFuture.isDone()) {
                 // There was an error executing the scheduled task, get it and 
log it
                 try {
-                    asyncTimeoutFuture.get();
+                    timeoutFuture.get();
                 } catch (InterruptedException | ExecutionException e) {
                     
getLog().error(sm.getString("abstractProtocolHandler.asyncTimeoutError"), e);
                 }
             }
-            asyncTimeoutFuture = getUtilityExecutor().scheduleAtFixedRate(
+            timeoutFuture = getUtilityExecutor().scheduleAtFixedRate(
                     new Runnable() {
                         @Override
                         public void run() {
@@ -619,9 +624,9 @@ public abstract class AbstractProtocol<S> implements 
ProtocolHandler,
     }
 
     protected void stopAsyncTimeout() {
-        if (asyncTimeoutFuture != null) {
-            asyncTimeoutFuture.cancel(false);
-            asyncTimeoutFuture = null;
+        if (timeoutFuture != null) {
+            timeoutFuture.cancel(false);
+            timeoutFuture = null;
         }
     }
 
@@ -664,7 +669,7 @@ public abstract class AbstractProtocol<S> implements 
ProtocolHandler,
             monitorFuture = null;
         }
         stopAsyncTimeout();
-        // Timeout any pending async request
+        // Timeout any waiting processor
         for (Processor processor : waitingProcessors) {
             processor.timeoutAsync(-1);
         }
@@ -770,12 +775,14 @@ public abstract class AbstractProtocol<S> implements 
ProtocolHandler,
                         processor, socket));
             }
 
-            // Async timeouts are calculated on a dedicated thread and then
+            // Timeouts are calculated on a dedicated thread and then
             // dispatched. Because of delays in the dispatch process, the
             // timeout may no longer be required. Check here and avoid
             // unnecessary processing.
-            if (SocketEvent.TIMEOUT == status && (processor == null ||
-                    !processor.isAsync() || 
!processor.checkAsyncTimeoutGeneration())) {
+            if (SocketEvent.TIMEOUT == status &&
+                    (processor == null ||
+                    !processor.isAsync() && !processor.isUpgrade() ||
+                    processor.isAsync() && 
!processor.checkAsyncTimeoutGeneration())) {
                 // This is effectively a NO-OP
                 return SocketState.OPEN;
             }
@@ -942,6 +949,7 @@ public abstract class AbstractProtocol<S> implements 
ProtocolHandler,
                     // to the poller if necessary.
                     if (status != SocketEvent.OPEN_WRITE) {
                         longPoll(wrapper, processor);
+                        getProtocol().addWaitingProcessor(processor);
                     }
                 } else if (state == SocketState.SUSPENDED) {
                     // Don't add sockets back to the poller.
diff --git a/java/org/apache/coyote/Processor.java 
b/java/org/apache/coyote/Processor.java
index 68edff5..c58431e 100644
--- a/java/org/apache/coyote/Processor.java
+++ b/java/org/apache/coyote/Processor.java
@@ -65,13 +65,17 @@ public interface Processor {
     boolean isAsync();
 
     /**
-     * Check this processor to see if the async timeout has expired and process
-     * a timeout if that is that case.
+     * Check this processor to see if the timeout has expired and process a
+     * timeout if that is that case.
+     * <p>
+     * Note: The name of this method originated with the Servlet 3.0
+     * asynchronous processing but evolved over time to represent a timeout 
that
+     * is triggered independently of the socket read/write timeouts.
      *
      * @param now The time (as returned by {@link System#currentTimeMillis()} 
to
-     *            use as the current time to determine whether the async 
timeout
-     *            has expired. If negative, the timeout will always be treated
-     *            as if it has expired.
+     *            use as the current time to determine whether the timeout has
+     *            expired. If negative, the timeout will always be treated as 
ifq
+     *            it has expired.
      */
     void timeoutAsync(long now);
 
diff --git a/java/org/apache/coyote/http11/upgrade/UpgradeProcessorBase.java 
b/java/org/apache/coyote/http11/upgrade/UpgradeProcessorBase.java
index b23f91d..a307299 100644
--- a/java/org/apache/coyote/http11/upgrade/UpgradeProcessorBase.java
+++ b/java/org/apache/coyote/http11/upgrade/UpgradeProcessorBase.java
@@ -91,13 +91,15 @@ public abstract class UpgradeProcessorBase extends 
AbstractProcessorLight implem
 
 
     @Override
-    public void timeoutAsync(long now) {
-        // NO-OP
+    public boolean checkAsyncTimeoutGeneration() {
+        return false;
     }
 
 
+    // ----------------- Processor methods that are NO-OP by default for 
upgrade
+
     @Override
-    public boolean checkAsyncTimeoutGeneration() {
-        return false;
+    public void timeoutAsync(long now) {
+        // NO-OP
     }
 }


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

Reply via email to