This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 6285e75 Improve fix to avoid deadlock reported on some
systems
6285e75 is described below
commit 6285e750e2b640fa9c59c11c09a753a36a5396b8
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Jan 3 17:24:59 2022 +0000
Improve fix to avoid deadlock reported on some systems
https://markmail.org/message/ldpjfdwpkmqc7ved
---
java/org/apache/coyote/http2/Http2UpgradeHandler.java | 14
++++++++++++--
java/org/apache/coyote/http2/Stream.java | 18
++++++++++--------
2 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 1195997..a2c87ed 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -345,7 +345,7 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
// continue reading frames
Stream stream =
getStream(se.getStreamId(), false);
if (stream == null) {
- sendStreamReset(se);
+ sendStreamReset(null, se);
} else {
stream.close(se);
}
@@ -535,7 +535,7 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
}
- void sendStreamReset(StreamException se) throws IOException {
+ void sendStreamReset(StreamStateMachine state, StreamException
se) throws IOException {
if (log.isDebugEnabled()) {
log.debug(sm.getString("upgradeHandler.rst.debug",
connectionId,
@@ -554,7 +554,17 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
// Payload
ByteUtil.setFourBytes(rstFrame, 9, se.getError().getCode());
+ // Need to update state atomically with the sending of the RST
+ // frame else other threads currently working with this stream
+ // may see the state change and send a RST frame before the RST
+ // frame triggered by this thread. If that happens the client
+ // may see out of order RST frames which may hard to follow if
+ // the client is unaware the RST frames may be received out of
+ // order.
synchronized (socketWrapper) {
+ if (state != null) {
+ state.sendReset();
+ }
socketWrapper.write(true, rstFrame, 0, rstFrame.length);
socketWrapper.flush(true);
}
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index fffd240..a4d6484 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -614,14 +614,16 @@ class Stream extends AbstractNonZeroStream
implements HeaderEmitter {
log.debug(sm.getString("stream.reset.send",
getConnectionId(), getIdAsString(),
se.getError()));
}
- // Sync ensures that if the call to sendReset()
triggers resets
- // in other threads, that the RST frame associated
with this
- // thread is sent before the RST frames associated
with those
- // threads.
- synchronized (state) {
- state.sendReset();
- handler.sendStreamReset(se);
- }
+
+ // Need to update state atomically with the sending
of the RST
+ // frame else other threads currently working with
this stream
+ // may see the state change and send a RST frame
before the RST
+ // frame triggered by this thread. If that happens
the client
+ // may see out of order RST frames which may hard to
follow if
+ // the client is unaware the RST frames may be
received out of
+ // order.
+ handler.sendStreamReset(state, se);
+
cancelAllocationRequests();
if (inputBuffer != null) {
inputBuffer.swallowUnread();
