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 8ba16e0 Improve fix to avoid deadlock reported on some systems
8ba16e0 is described below
commit 8ba16e09bfc58cdbac820790e8c425279f04cd98
Author: Mark Thomas <[email protected]>
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 ++++++++++--------
webapps/docs/changelog.xml | 5 +++++
3 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 1c522af..91abf18 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -351,7 +351,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);
}
@@ -534,7 +534,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,
@@ -553,7 +553,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 9f8a83e..1451a1b 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -655,14 +655,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();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 9f87ce2..e0e3d32 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -132,6 +132,11 @@
<bug>65757</bug>: Missing initial IO listener notification on Servlet
container dispatch to another container thread. (remm)
</fix>
+ <fix>
+ Improve the fix for RST frame ordering added in 10.0.14 to avoid a
+ potential deadlock on some systems in non-default configurations.
+ (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
