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(); --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org