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 2021/06/17 21:26:48 UTC

[tomcat] branch 10.0.x updated: Fix potential hang with concurrent reads or concurrent writes

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

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


The following commit(s) were added to refs/heads/10.0.x by this push:
     new e3e3993  Fix potential hang with concurrent reads or concurrent writes
e3e3993 is described below

commit e3e3993f70540b631509e66e2eca7091289e4388
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Jun 17 22:25:54 2021 +0100

    Fix potential hang with concurrent reads or concurrent writes
    
    Fot two threads, T1 and T2, both writing to the same connection, the
    sequence of events for a failure is as follows (line numbers all pre
    this commit):
    
    - T1 obtains the write semaphore (L1366)
    - T1 creates an OperationState and sets writeOperation (L1390)
    - the async write for T1 completes and the completion handler is called
    - T1's completion handler releases the semaphore (L1046)
    - T2 obtains the write semaphore (L1366)
    - T2 creates an OperationState and sets writeOperation (L1390)
    - T1's completion handler clears writeOperation (L1050)
    - the async write for T2 does not complete and the socket is added to
      the Poller
    - The Poller signals the socket is ready for write
    - The Poller finds writeOperation is null so performs a normal dispatch
      for write
    - The async write times out as it never receives the notification from
      the Poller
    
    The fix is to swap the order of clearing writeOperation and releasing
    the semaphore.
---
 java/org/apache/tomcat/util/net/SocketWrapperBase.java | 12 ++++++++++--
 webapps/docs/changelog.xml                             |  5 +++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/SocketWrapperBase.java b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
index 0a988aa..117bce8 100644
--- a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
+++ b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
@@ -1043,12 +1043,16 @@ public abstract class SocketWrapperBase<E> {
                 }
                 if (complete) {
                     boolean notify = false;
-                    state.semaphore.release();
                     if (state.read) {
                         readOperation = null;
                     } else {
                         writeOperation = null;
                     }
+                    // Semaphore must be released after [read|write]Operation is cleared
+                    // to ensure that the next thread to hold the semaphore hasn't
+                    // written a new value to [read|write]Operation by the time it is
+                    // cleared.
+                    state.semaphore.release();
                     if (state.block == BlockingMode.BLOCK && currentState != CompletionState.INLINE) {
                         notify = true;
                     } else {
@@ -1084,12 +1088,16 @@ public abstract class SocketWrapperBase<E> {
             }
             setError(ioe);
             boolean notify = false;
-            state.semaphore.release();
             if (state.read) {
                 readOperation = null;
             } else {
                 writeOperation = null;
             }
+            // Semaphore must be released after [read|write]Operation is cleared
+            // to ensure that the next thread to hold the semaphore hasn't
+            // written a new value to [read|write]Operation by the time it is
+            // cleared.
+            state.semaphore.release();
             if (state.block == BlockingMode.BLOCK) {
                 notify = true;
             } else {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 06c3315..b1c87fe 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -170,6 +170,11 @@
         buffer is now flushed (and the response committed if required) if the
         buffer is full and there is more data to write. (markt)
       </fix>
+      <fix>
+        Fix an issue where concurrent HTTP/2 writes (or concurrent reads) to the
+        same connection could hang and eventually timeout when async IO was
+        enabled (it is enabled by default). (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Web applications">

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