From caf46b0b0b7cebe7b7f44e385532764024ab0358 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Wed, 18 Oct 2023 18:14:12 +0200 Subject: [PATCH 1/3] #10519 ignore IllegalStateException thrown by flusher when closing Signed-off-by: Ludovic Orban --- .../org/eclipse/jetty/quic/common/QuicSession.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/jetty-quic/quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicSession.java b/jetty-quic/quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicSession.java index 48a0fb46bc2..df41a0b579c 100644 --- a/jetty-quic/quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicSession.java +++ b/jetty-quic/quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicSession.java @@ -401,8 +401,17 @@ public abstract class QuicSession extends ContainerLifeCycle if (LOG.isDebugEnabled()) LOG.debug("outward closing 0x{}/{} on {}", Long.toHexString(error), reason, this); quicheConnection.close(error, reason); - // Flushing will eventually forward the outward close to the connection. - flush(); + try + { + // Flushing will eventually forward the outward close to the connection. + flush(); + } + catch (IllegalStateException ise) + { + // Flusher already is in CLOSED state, nothing else to do. + if (LOG.isDebugEnabled()) + LOG.debug("IllegalStateException caught while flushing, flusher={} {}", flusher, this, ise); + } } private void finishOutwardClose(Throwable failure) From a8a8c8b9eb5de525c0db179cd2a316089cb789fb Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Wed, 18 Oct 2023 18:15:31 +0200 Subject: [PATCH 2/3] #10519 check if the stream is still in a usable state when quiche_conn_stream_send returns QUICHE_ERR_DONE Signed-off-by: Ludovic Orban --- .../ForeignIncubatorQuicheConnection.java | 7 ++++++- .../quiche/foreign/incubator/quiche_h.java | 18 ++++++++++++++++++ .../quic/quiche/jna/JnaQuicheConnection.java | 5 +++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/jetty-quic/quic-quiche/quic-quiche-foreign-incubator/src/main/java/org/eclipse/jetty/quic/quiche/foreign/incubator/ForeignIncubatorQuicheConnection.java b/jetty-quic/quic-quiche/quic-quiche-foreign-incubator/src/main/java/org/eclipse/jetty/quic/quiche/foreign/incubator/ForeignIncubatorQuicheConnection.java index e3f8057f5b7..cc119bfb3d0 100644 --- a/jetty-quic/quic-quiche/quic-quiche-foreign-incubator/src/main/java/org/eclipse/jetty/quic/quiche/foreign/incubator/ForeignIncubatorQuicheConnection.java +++ b/jetty-quic/quic-quiche/quic-quiche-foreign-incubator/src/main/java/org/eclipse/jetty/quic/quiche/foreign/incubator/ForeignIncubatorQuicheConnection.java @@ -29,8 +29,8 @@ import jdk.incubator.foreign.MemoryAddress; import jdk.incubator.foreign.MemorySegment; import jdk.incubator.foreign.ResourceScope; import org.eclipse.jetty.quic.quiche.Quiche; -import org.eclipse.jetty.quic.quiche.Quiche.quiche_error; import org.eclipse.jetty.quic.quiche.Quiche.quic_error; +import org.eclipse.jetty.quic.quiche.Quiche.quiche_error; import org.eclipse.jetty.quic.quiche.QuicheConfig; import org.eclipse.jetty.quic.quiche.QuicheConnection; import org.eclipse.jetty.util.BufferUtil; @@ -865,7 +865,12 @@ public class ForeignIncubatorQuicheConnection extends QuicheConnection } if (written == quiche_error.QUICHE_ERR_DONE) + { + int rc = quiche_h.quiche_conn_stream_writable(quicheConn, streamId, buffer.remaining()); + if (rc < 0) + throw new IOException("failed to write to stream " + streamId + "; quiche_err=" + quiche_error.errToString(rc)); return 0; + } if (written < 0L) throw new IOException("failed to write to stream " + streamId + "; quiche_err=" + quiche_error.errToString(written)); buffer.position((int)(buffer.position() + written)); diff --git a/jetty-quic/quic-quiche/quic-quiche-foreign-incubator/src/main/java/org/eclipse/jetty/quic/quiche/foreign/incubator/quiche_h.java b/jetty-quic/quic-quiche/quic-quiche-foreign-incubator/src/main/java/org/eclipse/jetty/quic/quiche/foreign/incubator/quiche_h.java index 9994b5bcfe6..2bfad6e253a 100644 --- a/jetty-quic/quic-quiche/quic-quiche-foreign-incubator/src/main/java/org/eclipse/jetty/quic/quiche/foreign/incubator/quiche_h.java +++ b/jetty-quic/quic-quiche/quic-quiche-foreign-incubator/src/main/java/org/eclipse/jetty/quic/quiche/foreign/incubator/quiche_h.java @@ -310,6 +310,12 @@ public class quiche_h FunctionDescriptor.of(C_LONG, C_POINTER, C_LONG, C_POINTER, C_LONG, C_CHAR) ); + private static final MethodHandle quiche_conn_stream_writable$MH = downcallHandle( + "quiche_conn_stream_writable", + "(Ljdk/incubator/foreign/MemoryAddress;JJ)I", + FunctionDescriptor.of(C_INT, C_POINTER, C_LONG, C_LONG) + ); + private static final MethodHandle quiche_conn_stream_recv$MH = downcallHandle( "quiche_conn_stream_recv", "(Ljdk/incubator/foreign/MemoryAddress;JLjdk/incubator/foreign/MemoryAddress;JLjdk/incubator/foreign/MemoryAddress;)J", @@ -670,6 +676,18 @@ public class quiche_h } } + public static int quiche_conn_stream_writable(MemoryAddress conn, long stream_id, long buf_len) + { + try + { + return (int) quiche_conn_stream_writable$MH.invokeExact(conn, stream_id, buf_len); + } + catch (Throwable ex) + { + throw new AssertionError("should not reach here", ex); + } + } + public static byte quiche_stream_iter_next(MemoryAddress conn, MemoryAddress stream_id) { try diff --git a/jetty-quic/quic-quiche/quic-quiche-jna/src/main/java/org/eclipse/jetty/quic/quiche/jna/JnaQuicheConnection.java b/jetty-quic/quic-quiche/quic-quiche-jna/src/main/java/org/eclipse/jetty/quic/quiche/jna/JnaQuicheConnection.java index 5a4f980df8a..7383aabe777 100644 --- a/jetty-quic/quic-quiche/quic-quiche-jna/src/main/java/org/eclipse/jetty/quic/quiche/jna/JnaQuicheConnection.java +++ b/jetty-quic/quic-quiche/quic-quiche-jna/src/main/java/org/eclipse/jetty/quic/quiche/jna/JnaQuicheConnection.java @@ -692,7 +692,12 @@ public class JnaQuicheConnection extends QuicheConnection throw new IOException("connection was released"); int written = LibQuiche.INSTANCE.quiche_conn_stream_send(quicheConn, new uint64_t(streamId), jnaBuffer(buffer), new size_t(buffer.remaining()), last).intValue(); if (written == quiche_error.QUICHE_ERR_DONE) + { + int rc = LibQuiche.INSTANCE.quiche_conn_stream_writable(quicheConn, new uint64_t(streamId), new size_t(buffer.remaining())); + if (rc < 0) + throw new IOException("failed to write to stream " + streamId + "; quiche_err=" + quiche_error.errToString(rc)); return 0; + } if (written < 0L) throw new IOException("failed to write to stream " + streamId + "; quiche_err=" + quiche_error.errToString(written)); buffer.position(buffer.position() + written); From 8b5deea657c730850e759040d93b98488ef8ea02 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Thu, 19 Oct 2023 17:44:27 +0200 Subject: [PATCH 3/3] #10519 do not close the flusher to avoid an ISE when iterating it during idle timeout Signed-off-by: Ludovic Orban --- .../jetty/quic/common/QuicSession.java | 30 ++++++------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/jetty-quic/quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicSession.java b/jetty-quic/quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicSession.java index df41a0b579c..d23d85d0efe 100644 --- a/jetty-quic/quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicSession.java +++ b/jetty-quic/quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicSession.java @@ -401,17 +401,8 @@ public abstract class QuicSession extends ContainerLifeCycle if (LOG.isDebugEnabled()) LOG.debug("outward closing 0x{}/{} on {}", Long.toHexString(error), reason, this); quicheConnection.close(error, reason); - try - { - // Flushing will eventually forward the outward close to the connection. - flush(); - } - catch (IllegalStateException ise) - { - // Flusher already is in CLOSED state, nothing else to do. - if (LOG.isDebugEnabled()) - LOG.debug("IllegalStateException caught while flushing, flusher={} {}", flusher, this, ise); - } + // Flushing will eventually forward the outward close to the connection. + flush(); } private void finishOutwardClose(Throwable failure) @@ -419,7 +410,6 @@ public abstract class QuicSession extends ContainerLifeCycle try { endPoints.clear(); - flusher.close(); getQuicConnection().outwardClose(this, failure); } finally @@ -464,13 +454,6 @@ public abstract class QuicSession extends ContainerLifeCycle }; } - @Override - public void close() - { - super.close(); - timeout.destroy(); - } - @Override protected Action process() throws IOException { @@ -523,8 +506,7 @@ public abstract class QuicSession extends ContainerLifeCycle { if (LOG.isDebugEnabled()) LOG.debug("connection closed {}", QuicSession.this); - byteBufferPool.release(cipherBuffer); - finishOutwardClose(new ClosedChannelException()); + finish(new ClosedChannelException()); } @Override @@ -532,8 +514,14 @@ public abstract class QuicSession extends ContainerLifeCycle { if (LOG.isDebugEnabled()) LOG.debug("failed to write cipher bytes, closing session on {}", QuicSession.this, failure); + finish(failure); + } + + private void finish(Throwable failure) + { byteBufferPool.release(cipherBuffer); finishOutwardClose(failure); + timeout.destroy(); } }