You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2019/04/30 09:05:24 UTC
[tomcat] branch master updated: Async version of HTTP/2 preface and
initial frame read
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new 50fbf98 Async version of HTTP/2 preface and initial frame read
50fbf98 is described below
commit 50fbf985f1a25d8d4f9fd6d717c2cce87aec5946
Author: remm <re...@apache.org>
AuthorDate: Tue Apr 30 11:05:14 2019 +0200
Async version of HTTP/2 preface and initial frame read
The benefit is to avoid blocking reads (readFrame(true) remains handled
but is not going to be used in practice). This seems to work properly
for me.
---
java/org/apache/coyote/http2/Http2AsyncParser.java | 94 +++++++++++++++++++---
.../coyote/http2/Http2AsyncUpgradeHandler.java | 16 +++-
java/org/apache/coyote/http2/Http2Parser.java | 6 +-
.../apache/coyote/http2/Http2UpgradeHandler.java | 9 ++-
webapps/docs/changelog.xml | 4 +
5 files changed, 112 insertions(+), 17 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2AsyncParser.java b/java/org/apache/coyote/http2/Http2AsyncParser.java
index 5438b40..b00c092 100644
--- a/java/org/apache/coyote/http2/Http2AsyncParser.java
+++ b/java/org/apache/coyote/http2/Http2AsyncParser.java
@@ -21,6 +21,9 @@ import java.nio.ByteBuffer;
import java.nio.channels.CompletionHandler;
import java.util.concurrent.TimeUnit;
+import javax.servlet.http.WebConnection;
+
+import org.apache.coyote.ProtocolException;
import org.apache.tomcat.util.net.SocketEvent;
import org.apache.tomcat.util.net.SocketWrapperBase;
import org.apache.tomcat.util.net.SocketWrapperBase.BlockingMode;
@@ -43,18 +46,85 @@ class Http2AsyncParser extends Http2Parser {
@Override
+ void readConnectionPreface(WebConnection webConnection, Stream stream) throws Http2Exception {
+ byte[] data = new byte[CLIENT_PREFACE_START.length];
+ socketWrapper.read(BlockingMode.NON_BLOCK, socketWrapper.getReadTimeout(), TimeUnit.MILLISECONDS, null,
+ SocketWrapperBase.COMPLETE_READ_WITH_COMPLETION, new CompletionHandler<Long, Void>() {
+ @Override
+ public void completed(Long result, Void attachment) {
+ for (int i = 0; i < CLIENT_PREFACE_START.length; i++) {
+ if (CLIENT_PREFACE_START[i] != data[i]) {
+ failed(new ProtocolException(sm.getString("http2Parser.preface.invalid")), null);
+ return;
+ }
+ }
+ // Must always be followed by a settings frame
+ try {
+ ByteBuffer header = ByteBuffer.allocate(9);
+ ByteBuffer framePaylod = ByteBuffer.allocate(input.getMaxFrameSize());
+ FrameCompletionHandler handler = new FrameCompletionHandler(FrameType.SETTINGS, header, framePaylod) {
+ @Override
+ public void completed(Long result, Void attachment) {
+ if (streamException || error == null) {
+ ByteBuffer payload = buffers[1];
+ payload.flip();
+ try {
+ if (streamException) {
+ swallow(streamId, payloadSize, false, payload);
+ } else {
+ switch (frameType) {
+ case SETTINGS:
+ readSettingsFrame(flags, payloadSize, payload);
+ break;
+ default:
+ // Should never happen as the frame has been validated as SETTINGS
+ throw new StreamException(sm.getString("http2Parser.processFrame.unexpectedType",
+ FrameType.SETTINGS, frameType), Http2Error.PROTOCOL_ERROR, streamId);
+ }
+ }
+ } catch (RuntimeException | IOException | Http2Exception e) {
+ error = e;
+ }
+ if (payload.hasRemaining()) {
+ socketWrapper.unRead(payload);
+ }
+ }
+ // Continue processing the intial parts of the connection
+ upgradeHandler.processConnectionCallback(webConnection, stream);
+ if (state == CompletionState.DONE) {
+ // The call was not completed inline, so must start reading new frames
+ // or process the stream exception
+ upgradeHandler.upgradeDispatch(SocketEvent.OPEN_READ);
+ }
+ }};
+ socketWrapper.read(BlockingMode.NON_BLOCK, socketWrapper.getReadTimeout(), TimeUnit.MILLISECONDS,
+ null, handler, handler, header, framePaylod);
+ } catch (Exception e) {
+ failed(e, null);
+ }
+ }
+ @Override
+ public void failed(Throwable exc, Void attachment) {
+ if (exc instanceof IOException) {
+ error = new ProtocolException(sm.getString("http2Parser.preface.invalid"));
+ } else {
+ error = exc;
+ }
+ upgradeHandler.upgradeDispatch(SocketEvent.ERROR);
+ }
+ }, ByteBuffer.wrap(data));
+ }
+
+
+ @Override
protected boolean readFrame(boolean block, FrameType expected)
throws IOException, Http2Exception {
- if (block) {
- // Only used when reading the connection preface
- return super.readFrame(block, expected);
- }
handleAsyncException();
ByteBuffer header = ByteBuffer.allocate(9);
ByteBuffer framePaylod = ByteBuffer.allocate(input.getMaxFrameSize());
FrameCompletionHandler handler = new FrameCompletionHandler(expected, header, framePaylod);
CompletionState state =
- socketWrapper.read(BlockingMode.NON_BLOCK, socketWrapper.getReadTimeout(), TimeUnit.MILLISECONDS, null, handler, handler, header, framePaylod);
+ socketWrapper.read(block ? BlockingMode.BLOCK : BlockingMode.NON_BLOCK, socketWrapper.getReadTimeout(), TimeUnit.MILLISECONDS, null, handler, handler, header, framePaylod);
if (state == CompletionState.ERROR || state == CompletionState.INLINE) {
handleAsyncException();
return true;
@@ -84,13 +154,13 @@ class Http2AsyncParser extends Http2Parser {
private boolean validated = false;
private final FrameType expected;
- private final ByteBuffer[] buffers;
- private int payloadSize;
- private FrameType frameType;
- private int flags;
- private int streamId;
- private boolean streamException = false;
- private CompletionState state = null;
+ protected final ByteBuffer[] buffers;
+ protected int payloadSize;
+ protected FrameType frameType;
+ protected int flags;
+ protected int streamId;
+ protected boolean streamException = false;
+ protected CompletionState state = null;
private FrameCompletionHandler(FrameType expected, ByteBuffer... buffers) {
this.expected = expected;
diff --git a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
index f4559a3..fde2bb6 100644
--- a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
@@ -26,6 +26,8 @@ import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;
+import javax.servlet.http.WebConnection;
+
import org.apache.coyote.Adapter;
import org.apache.coyote.ProtocolException;
import org.apache.coyote.Request;
@@ -42,7 +44,7 @@ public class Http2AsyncUpgradeHandler extends Http2UpgradeHandler {
public Http2AsyncUpgradeHandler(Http2Protocol protocol, Adapter adapter,
Request coyoteRequest) {
- super (protocol, adapter, coyoteRequest);
+ super(protocol, adapter, coyoteRequest);
}
private CompletionHandler<Long, Void> errorCompletion = new CompletionHandler<Long, Void>() {
@@ -84,6 +86,18 @@ public class Http2AsyncUpgradeHandler extends Http2UpgradeHandler {
}
@Override
+ protected void processConnection(WebConnection webConnection,
+ Stream stream) {
+ // The end of the processing will instead be an async callback
+ }
+
+ void processConnectionCallback(WebConnection webConnection,
+ Stream stream) {
+ super.processConnection(webConnection, stream);
+ }
+
+
+ @Override
protected void writeSettings() {
// Send the initial settings frame
socketWrapper.write(BlockingMode.SEMI_BLOCK, protocol.getWriteTimeout(),
diff --git a/java/org/apache/coyote/http2/Http2Parser.java b/java/org/apache/coyote/http2/Http2Parser.java
index f5e1986..e99d3ff 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -20,6 +20,8 @@ import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
+import javax.servlet.http.WebConnection;
+
import org.apache.coyote.ProtocolException;
import org.apache.coyote.http2.HpackDecoder.HeaderEmitter;
import org.apache.juli.logging.Log;
@@ -644,8 +646,10 @@ class Http2Parser {
/**
* Read and validate the connection preface from input using blocking IO.
+ * @param webConnection The connection
+ * @param stream The current stream
*/
- void readConnectionPreface() throws Http2Exception {
+ void readConnectionPreface(WebConnection webConnection, Stream stream) throws Http2Exception {
byte[] data = new byte[CLIENT_PREFACE_START.length];
try {
input.fill(true, data);
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 349a4c8..a787349 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -231,7 +231,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
// Make sure the client has sent a valid connection preface before we
// send the response to the original request over HTTP/2.
try {
- parser.readConnectionPreface();
+ parser.readConnectionPreface(webConnection, stream);
} catch (Http2Exception e) {
String msg = sm.getString("upgradeHandler.invalidPreface", connectionId);
if (log.isDebugEnabled()) {
@@ -243,6 +243,10 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
log.debug(sm.getString("upgradeHandler.prefaceReceived", connectionId));
}
+ processConnection(webConnection, stream);
+ }
+
+ protected void processConnection(WebConnection webConnection, Stream stream) {
// Send a ping to get an idea of round trip time as early as possible
try {
pingManager.sendPing(true);
@@ -255,13 +259,12 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
}
}
-
protected Http2Parser getParser(String connectionId) {
return new Http2Parser(connectionId, this, this);
}
- private void processStreamOnContainerThread(Stream stream) {
+ protected void processStreamOnContainerThread(Stream stream) {
StreamProcessor streamProcessor = new StreamProcessor(this, stream, adapter, socketWrapper);
streamProcessor.setSslSupport(sslSupport);
processStreamOnContainerThread(streamProcessor, SocketEvent.OPEN_READ);
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index b25cdf6..cd07b3e 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -142,6 +142,10 @@
The async HTTP/2 frame parser should tolerate concurrency so clearing
shared buffers before attempting a read is not possible. (remm)
</fix>
+ <update>
+ Update the HTTP/2 connection preface and initial frame reading to be
+ asynchronous instead of blocking IO. (remm)
+ </update>
</changelog>
</subsection>
<subsection name="Other">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org