This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 953ff9c Fix potential hang with concurrent reads or concurrent writes
953ff9c is described below
commit 953ff9c9d1d83dc7e4608c7608560ab5f72e5fa5
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 07580f9..008715e 100644
--- a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
+++ b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
@@ -1073,12 +1073,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 {
@@ -1114,12 +1118,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 e9eeecd..8fc330a 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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
