Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Issue #6728 - QUIC and HTTP/3 

- Fixed parsing of HEADERS frames.
- Fixed locking in QpackEncoder.
- Fixed creation of QuicStreamEndPoints.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
Simone Bordet 2021-11-05 18:03:35 +01:00
parent cd1343fd6c
commit a7ec4ff525
5 changed files with 103 additions and 89 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/internal/HTTP3StreamConnection.java
View File

@ -255,7 +255,7 @@ public abstract class HTTP3StreamConnection extends AbstractConnection
{
buffer.release();
if (LOG.isDebugEnabled())
LOG.debug("retained released {}", buffer);
LOG.debug("released retained {}", buffer);
}
public void demand()

6
jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/internal/parser/HeadersBodyParser.java
View File

@ -23,7 +23,6 @@ import org.eclipse.jetty.http3.frames.HeadersFrame;
import org.eclipse.jetty.http3.internal.HTTP3ErrorCode;
import org.eclipse.jetty.http3.qpack.QpackDecoder;
import org.eclipse.jetty.http3.qpack.QpackException;
import org.eclipse.jetty.util.BufferUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -72,7 +71,9 @@ public class HeadersBodyParser extends BodyParser
{
// Copy and accumulate the buffer.
length -= remaining;
ByteBuffer copy = BufferUtil.copy(buffer);
ByteBuffer copy = buffer.isDirect() ? ByteBuffer.allocateDirect(remaining) : ByteBuffer.allocate(remaining);
copy.put(buffer);
copy.flip();
byteBuffers.add(copy);
return Result.NO_FRAME;
}
@ -96,6 +97,7 @@ public class HeadersBodyParser extends BodyParser
byteBuffers.add(slice);
int capacity = byteBuffers.stream().mapToInt(ByteBuffer::remaining).sum();
encoded = byteBuffers.stream().reduce(ByteBuffer.allocate(capacity), ByteBuffer::put);
encoded.flip();
byteBuffers.clear();
}

7
jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java
View File

@ -40,6 +40,7 @@ import org.eclipse.jetty.http3.qpack.internal.table.Entry;
import org.eclipse.jetty.http3.qpack.internal.util.NBitIntegerEncoder;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.component.Dumpable;
import org.eclipse.jetty.util.thread.AutoLock;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -86,6 +87,7 @@ public class QpackEncoder implements Dumpable
HttpHeader.SET_COOKIE,
HttpHeader.SET_COOKIE2);
private final AutoLock lock = new AutoLock();
private final List<Instruction> _instructions = new ArrayList<>();
private final Instruction.Handler _handler;
private final QpackContext _context;
@ -128,6 +130,8 @@ public class QpackEncoder implements Dumpable
* @throws QpackException if there was an error with the QPACK compression.
*/
public void encode(ByteBuffer buffer, long streamId, MetaData metadata) throws QpackException
{
try (AutoLock l = lock.lock())
{
if (LOG.isDebugEnabled())
LOG.debug("Encoding: streamId={}, metadata={}", streamId, metadata);
@ -208,6 +212,7 @@ public class QpackEncoder implements Dumpable
throw new QpackException.SessionException(H3_GENERAL_PROTOCOL_ERROR, "compression_error", t);
}
}
}
/**
* Parse instructions from the Decoder stream. The Decoder stream carries an unframed sequence of instructions from
@ -219,7 +224,7 @@ public class QpackEncoder implements Dumpable
*/
public void parseInstructions(ByteBuffer buffer) throws QpackException
{
try
try (AutoLock l = lock.lock())
{
while (BufferUtil.hasContent(buffer))
{

20
jetty-quic/quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicSession.java
View File

@ -25,6 +25,7 @@ import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.Executor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicLong;
import java.util.function.Consumer;
@ -379,17 +380,22 @@ public abstract class QuicSession extends ContainerLifeCycle
public QuicStreamEndPoint getOrCreateStreamEndPoint(long streamId, Consumer<QuicStreamEndPoint> consumer)
{
QuicStreamEndPoint endPoint = endPoints.compute(streamId, (id, quicStreamEndPoint) ->
{
if (quicStreamEndPoint == null)
AtomicBoolean created = new AtomicBoolean();
QuicStreamEndPoint endPoint = endPoints.computeIfAbsent(streamId, id ->
{
if (LOG.isDebugEnabled())
LOG.debug("creating endpoint for stream #{} for {}", id, this);
quicStreamEndPoint = newQuicStreamEndPoint(streamId);
consumer.accept(quicStreamEndPoint);
}
return quicStreamEndPoint;
QuicStreamEndPoint result = newQuicStreamEndPoint(id);
created.set(true);
return result;
});
// The consumer must be executed outside the Map.compute() above,
// since it may take a long time and it may be re-entrant, causing the
// creation of two QuicStreamEndPoint objects for the same stream id.
if (created.get())
consumer.accept(endPoint);
if (LOG.isDebugEnabled())
LOG.debug("returning {} for {}", endPoint, this);
return endPoint;

15
tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java
View File

@ -96,7 +96,7 @@ public class HttpClientLoadTest extends AbstractTest<HttpClientLoadTest.LoadTran
}
// Re-run after warmup
iterations = 1_000;
iterations = 500;
for (int i = 0; i < runs; ++i)
{
run(transport, iterations);
@ -140,7 +140,7 @@ public class HttpClientLoadTest extends AbstractTest<HttpClientLoadTest.LoadTran
});
int runs = 1;
int iterations = 256;
int iterations = 64;
IntStream.range(0, 16).parallel().forEach(i ->
IntStream.range(0, runs).forEach(j ->
run(transport, iterations)));
@ -155,13 +155,14 @@ public class HttpClientLoadTest extends AbstractTest<HttpClientLoadTest.LoadTran
// Dumps the state of the client if the test takes too long
Thread testThread = Thread.currentThread();
long maxTime = Math.max(5000, (long)iterations * factor);
Scheduler.Task task = scenario.client.getScheduler().schedule(() ->
{
logger.warn("Interrupting test, it is taking too long{}{}{}{}",
logger.warn("Interrupting test, it is taking too long (maxTime={} ms){}{}{}{}", maxTime,
System.lineSeparator(), scenario.server.dump(),
System.lineSeparator(), scenario.client.dump());
testThread.interrupt();
}, Math.max(5000, (long)iterations * factor), TimeUnit.MILLISECONDS);
}, maxTime, TimeUnit.MILLISECONDS);
long begin = System.nanoTime();
for (int i = 0; i < iterations; ++i)
@ -169,7 +170,6 @@ public class HttpClientLoadTest extends AbstractTest<HttpClientLoadTest.LoadTran
test(latch, failures);
// test("http", "localhost", "GET", false, false, 64 * 1024, false, latch, failures);
}
assertTrue(await(latch, iterations, TimeUnit.SECONDS));
long end = System.nanoTime();
task.cancel();
long elapsed = TimeUnit.NANOSECONDS.toMillis(end - begin);
@ -282,9 +282,10 @@ public class HttpClientLoadTest extends AbstractTest<HttpClientLoadTest.LoadTran
latch.countDown();
}
});
if (!await(requestLatch, 5, TimeUnit.SECONDS))
int maxTime = 5000;
if (!await(requestLatch, maxTime, TimeUnit.MILLISECONDS))
{
logger.warn("Request {} took too long{}{}{}{}", requestId,
logger.warn("Request {} took too long (maxTime={} ms){}{}{}{}", requestId, maxTime,
System.lineSeparator(), scenario.server.dump(),
System.lineSeparator(), scenario.client.dump());
}