From 6d0bddc07af28fde5513efe445f17281fb116661 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Mon, 24 Jun 2024 14:38:28 +0200 Subject: [PATCH 1/9] #11932 make succeeded and failed in ICB final + introduce onSuccess Signed-off-by: Ludovic Orban --- .../jetty/docs/programming/ContentDocs.java | 9 +-------- .../jetty/docs/programming/ContentDocs.java | 9 +-------- .../jetty/client/transport/HttpSender.java | 4 +--- .../transport/internal/HttpSenderOverHTTP.java | 11 ++--------- .../eclipse/jetty/fcgi/generator/Flusher.java | 3 +-- .../jetty/http2/internal/HTTP2Flusher.java | 3 +-- .../jetty/http2/tests/RawHTTP2ProxyTest.java | 16 ++++++---------- .../org/eclipse/jetty/http3/ControlFlusher.java | 4 +--- .../eclipse/jetty/http3/InstructionFlusher.java | 4 +--- .../org/eclipse/jetty/http3/MessageFlusher.java | 10 ++++------ .../jetty/io/content/BufferedContentSink.java | 3 +-- .../jetty/quic/common/QuicConnection.java | 13 +++---------- .../eclipse/jetty/quic/common/QuicSession.java | 3 +-- .../jetty/server/handler/ConnectHandler.java | 8 +------- .../client/transport/CustomTransportTest.java | 13 +++---------- .../eclipse/jetty/util/IteratingCallback.java | 17 +++++++++++++++++ .../jetty/ee10/proxy/AsyncProxyServlet.java | 5 ++--- .../jetty/ee9/proxy/AsyncProxyServlet.java | 5 ++--- 18 files changed, 49 insertions(+), 91 deletions(-) diff --git a/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/ContentDocs.java b/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/ContentDocs.java index fad96cac277..d406ee1d3bd 100644 --- a/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/ContentDocs.java +++ b/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/ContentDocs.java @@ -347,17 +347,10 @@ public class ContentDocs } @Override - public void succeeded() + protected void onSuccess() { // After every successful write, release the chunk. chunk.release(); - super.succeeded(); - } - - @Override - public void failed(Throwable x) - { - super.failed(x); } @Override diff --git a/documentation/jetty/modules/code/examples/src/main/java/org/eclipse/jetty/docs/programming/ContentDocs.java b/documentation/jetty/modules/code/examples/src/main/java/org/eclipse/jetty/docs/programming/ContentDocs.java index fad96cac277..d406ee1d3bd 100644 --- a/documentation/jetty/modules/code/examples/src/main/java/org/eclipse/jetty/docs/programming/ContentDocs.java +++ b/documentation/jetty/modules/code/examples/src/main/java/org/eclipse/jetty/docs/programming/ContentDocs.java @@ -347,17 +347,10 @@ public class ContentDocs } @Override - public void succeeded() + protected void onSuccess() { // After every successful write, release the chunk. chunk.release(); - super.succeeded(); - } - - @Override - public void failed(Throwable x) - { - super.failed(x); } @Override diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpSender.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpSender.java index c60607572a7..551a210ae0e 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpSender.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpSender.java @@ -543,7 +543,7 @@ public abstract class HttpSender } @Override - public void succeeded() + protected void onSuccess() { boolean proceed = true; if (committed) @@ -588,8 +588,6 @@ public abstract class HttpSender // There was some concurrent error, terminate. complete = true; } - - super.succeeded(); } @Override diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java index 8fff51d6454..1f6bc4a3f75 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java @@ -235,17 +235,9 @@ public class HttpSenderOverHTTP extends HttpSender } @Override - public void succeeded() + protected void onSuccess() { release(); - super.succeeded(); - } - - @Override - public void failed(Throwable x) - { - release(); - super.failed(x); } @Override @@ -259,6 +251,7 @@ public class HttpSenderOverHTTP extends HttpSender protected void onCompleteFailure(Throwable cause) { super.onCompleteFailure(cause); + release(); callback.failed(cause); } diff --git a/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/generator/Flusher.java b/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/generator/Flusher.java index 0093517653a..9c64ffb7794 100644 --- a/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/generator/Flusher.java +++ b/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/generator/Flusher.java @@ -101,12 +101,11 @@ public class Flusher } @Override - public void succeeded() + protected void onSuccess() { if (active != null) active.succeeded(); active = null; - super.succeeded(); } @Override diff --git a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/internal/HTTP2Flusher.java b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/internal/HTTP2Flusher.java index b7a67c9fbe3..36b11c794dd 100644 --- a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/internal/HTTP2Flusher.java +++ b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/internal/HTTP2Flusher.java @@ -294,7 +294,7 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable } @Override - public void succeeded() + protected void onSuccess() { if (LOG.isDebugEnabled()) LOG.debug("Written {} buffers - entries processed/pending {}/{}: {}/{}", @@ -304,7 +304,6 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable processedEntries, pendingEntries); finish(); - super.succeeded(); } private void finish() diff --git a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/RawHTTP2ProxyTest.java b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/RawHTTP2ProxyTest.java index 711e705ba55..ff2e8f3fc5a 100644 --- a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/RawHTTP2ProxyTest.java +++ b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/RawHTTP2ProxyTest.java @@ -515,17 +515,15 @@ public class RawHTTP2ProxyTest } @Override - public void succeeded() + protected void onSuccess() { frameInfo.callback.succeeded(); - super.succeeded(); } @Override - public void failed(Throwable failure) + protected void onCompleteFailure(Throwable cause) { - frameInfo.callback.failed(failure); - super.failed(failure); + frameInfo.callback.failed(cause); } @Override @@ -671,17 +669,15 @@ public class RawHTTP2ProxyTest } @Override - public void succeeded() + protected void onSuccess() { frameInfo.callback.succeeded(); - super.succeeded(); } @Override - public void failed(Throwable failure) + protected void onCompleteFailure(Throwable cause) { - frameInfo.callback.failed(failure); - super.failed(failure); + frameInfo.callback.failed(cause); } private void offer(Stream stream, Frame frame, Callback callback) diff --git a/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/ControlFlusher.java b/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/ControlFlusher.java index 3e4aa1ca195..deee66c03b7 100644 --- a/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/ControlFlusher.java +++ b/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/ControlFlusher.java @@ -108,7 +108,7 @@ public class ControlFlusher extends IteratingCallback } @Override - public void succeeded() + protected void onSuccess() { if (LOG.isDebugEnabled()) LOG.debug("succeeded to write {} on {}", entries, this); @@ -119,8 +119,6 @@ public class ControlFlusher extends IteratingCallback entries.clear(); invocationType = InvocationType.NON_BLOCKING; - - super.succeeded(); } @Override diff --git a/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/InstructionFlusher.java b/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/InstructionFlusher.java index 304d24975ed..6c826c707a4 100644 --- a/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/InstructionFlusher.java +++ b/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/InstructionFlusher.java @@ -104,14 +104,12 @@ public class InstructionFlusher extends IteratingCallback } @Override - public void succeeded() + protected void onSuccess() { if (LOG.isDebugEnabled()) LOG.debug("succeeded to write {} buffers on {}", accumulator.getByteBuffers().size(), this); accumulator.release(); - - super.succeeded(); } @Override diff --git a/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/MessageFlusher.java b/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/MessageFlusher.java index 59427685526..8f0ae115d81 100644 --- a/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/MessageFlusher.java +++ b/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/MessageFlusher.java @@ -89,7 +89,7 @@ public class MessageFlusher extends IteratingCallback } @Override - public void succeeded() + protected void onSuccess() { if (LOG.isDebugEnabled()) LOG.debug("succeeded to write {} on {}", entry, this); @@ -98,19 +98,17 @@ public class MessageFlusher extends IteratingCallback entry.callback.succeeded(); entry = null; - - super.succeeded(); } @Override - public void failed(Throwable x) + protected void onCompleteFailure(Throwable cause) { if (LOG.isDebugEnabled()) - LOG.debug("failed to write {} on {}", entry, this, x); + LOG.debug("failed to write {} on {}", entry, this, cause); accumulator.release(); - entry.callback.failed(x); + entry.callback.failed(cause); entry = null; // Continue the iteration. diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/BufferedContentSink.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/BufferedContentSink.java index 26a0d972395..3035cd3c8f4 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/BufferedContentSink.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/BufferedContentSink.java @@ -287,13 +287,12 @@ public class BufferedContentSink implements Content.Sink } @Override - public void succeeded() + protected void onSuccess() { _buffer = null; Callback callback = _callback; _callback = null; callback.succeeded(); - super.succeeded(); } @Override diff --git a/jetty-core/jetty-quic/jetty-quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicConnection.java b/jetty-core/jetty-quic/jetty-quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicConnection.java index c5ff003db1b..722d494dbea 100644 --- a/jetty-core/jetty-quic/jetty-quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicConnection.java +++ b/jetty-core/jetty-quic/jetty-quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicConnection.java @@ -372,17 +372,9 @@ public abstract class QuicConnection extends AbstractConnection } @Override - public void succeeded() + protected void onSuccess() { entry.callback.succeeded(); - super.succeeded(); - } - - @Override - public void failed(Throwable x) - { - entry.callback.failed(x); - super.failed(x); } @Override @@ -394,10 +386,11 @@ public abstract class QuicConnection extends AbstractConnection @Override protected void onCompleteFailure(Throwable cause) { + entry.callback.failed(cause); QuicConnection.this.close(); } - private class Entry + private static class Entry { private final Callback callback; private final SocketAddress address; diff --git a/jetty-core/jetty-quic/jetty-quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicSession.java b/jetty-core/jetty-quic/jetty-quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicSession.java index 8f998fa8743..75ee111e659 100644 --- a/jetty-core/jetty-quic/jetty-quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicSession.java +++ b/jetty-core/jetty-quic/jetty-quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicSession.java @@ -521,12 +521,11 @@ public abstract class QuicSession extends ContainerLifeCycle } @Override - public void succeeded() + protected void onSuccess() { if (LOG.isDebugEnabled()) LOG.debug("written cipher bytes on {}", QuicSession.this); cipherBuffer.release(); - super.succeeded(); } @Override diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ConnectHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ConnectHandler.java index 76082dbba5f..51fdb7e1011 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ConnectHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ConnectHandler.java @@ -760,17 +760,11 @@ public class ConnectHandler extends Handler.Wrapper } @Override - public void succeeded() + protected void onSuccess() { if (LOG.isDebugEnabled()) LOG.debug("Wrote {} bytes {}", filled, TunnelConnection.this); buffer.release(); - super.succeeded(); - } - - @Override - protected void onCompleteSuccess() - { } @Override diff --git a/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/CustomTransportTest.java b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/CustomTransportTest.java index ed8e2b9c483..a3a7ecf9a46 100644 --- a/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/CustomTransportTest.java +++ b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/CustomTransportTest.java @@ -237,7 +237,8 @@ public class CustomTransportTest channels.put(channel.id, channel); // Register for read interest with the EndPoint. - endPoint.fillInterested(new EndPointToChannelCallback(channel)); + EndPointToChannelCallback endPointToChannelCallback = new EndPointToChannelCallback(channel); + endPoint.fillInterested(Callback.from(endPointToChannelCallback::iterate)); } // Called when there data to read from the Gateway on the given Channel. @@ -322,18 +323,10 @@ public class CustomTransportTest endPoint.fillInterested(this); return Action.IDLE; } - channel.write(this, buffer); + channel.write(Callback.from(this::iterate), buffer); return Action.SCHEDULED; } - @Override - public void succeeded() - { - // There is data to read from the EndPoint. - // Iterate to read it and send it to the Gateway. - iterate(); - } - @Override protected void onCompleteSuccess() { diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java index 9ada006fab6..76d3bff375c 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java @@ -167,6 +167,13 @@ public abstract class IteratingCallback implements Callback */ protected abstract Action process() throws Throwable; + /** + * Invoked when one task has completed successfully. + */ + protected void onSuccess() + { + } + /** * Invoked when the overall task has completed successfully. * @@ -239,6 +246,7 @@ public abstract class IteratingCallback implements Callback boolean notifyCompleteSuccess = false; Throwable notifyCompleteFailure = null; + boolean callOnSuccess = false; // While we are processing processing: while (true) @@ -247,6 +255,11 @@ public abstract class IteratingCallback implements Callback Action action = null; try { + if (callOnSuccess) + { + onSuccess(); + callOnSuccess = false; + } action = process(); } catch (Throwable x) @@ -309,6 +322,7 @@ public abstract class IteratingCallback implements Callback throw new IllegalStateException(String.format("%s[action=%s]", this, action)); // we lost the race, so we have to keep processing _state = State.PROCESSING; + callOnSuccess = true; continue; } @@ -374,7 +388,10 @@ public abstract class IteratingCallback implements Callback } } if (process) + { + onSuccess(); processing(); + } } /** diff --git a/jetty-ee10/jetty-ee10-proxy/src/main/java/org/eclipse/jetty/ee10/proxy/AsyncProxyServlet.java b/jetty-ee10/jetty-ee10-proxy/src/main/java/org/eclipse/jetty/ee10/proxy/AsyncProxyServlet.java index 3a4558e8b1d..11e048ac823 100644 --- a/jetty-ee10/jetty-ee10-proxy/src/main/java/org/eclipse/jetty/ee10/proxy/AsyncProxyServlet.java +++ b/jetty-ee10/jetty-ee10-proxy/src/main/java/org/eclipse/jetty/ee10/proxy/AsyncProxyServlet.java @@ -192,10 +192,9 @@ public class AsyncProxyServlet extends ProxyServlet } @Override - public void failed(Throwable x) + protected void onCompleteFailure(Throwable cause) { - super.failed(x); - onError(x); + onError(cause); } } diff --git a/jetty-ee9/jetty-ee9-proxy/src/main/java/org/eclipse/jetty/ee9/proxy/AsyncProxyServlet.java b/jetty-ee9/jetty-ee9-proxy/src/main/java/org/eclipse/jetty/ee9/proxy/AsyncProxyServlet.java index 9c9f87e8255..321da55fd15 100644 --- a/jetty-ee9/jetty-ee9-proxy/src/main/java/org/eclipse/jetty/ee9/proxy/AsyncProxyServlet.java +++ b/jetty-ee9/jetty-ee9-proxy/src/main/java/org/eclipse/jetty/ee9/proxy/AsyncProxyServlet.java @@ -192,10 +192,9 @@ public class AsyncProxyServlet extends ProxyServlet } @Override - public void failed(Throwable x) + protected void onCompleteFailure(Throwable cause) { - super.failed(x); - onError(x); + onError(cause); } } From 25bbdb5efcded43fb69312e002c1be1bfeaca003 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Tue, 25 Jun 2024 10:14:16 +0200 Subject: [PATCH 2/9] #11932 refactor H3 message flusher to have ICB succeeded and failed final Signed-off-by: Ludovic Orban --- .../eclipse/jetty/http3/MessageFlusher.java | 33 ++++++++++++++----- .../eclipse/jetty/util/IteratingCallback.java | 4 +-- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/MessageFlusher.java b/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/MessageFlusher.java index 8f0ae115d81..f450bfaf0ec 100644 --- a/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/MessageFlusher.java +++ b/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/MessageFlusher.java @@ -75,7 +75,7 @@ public class MessageFlusher extends IteratingCallback return Action.SCHEDULED; } - int generated = generator.generate(accumulator, entry.endPoint.getStreamId(), frame, this::failed); + int generated = generator.generate(accumulator, entry.endPoint.getStreamId(), frame, this::onGenerateFailure); if (generated < 0) return Action.SCHEDULED; @@ -88,6 +88,20 @@ public class MessageFlusher extends IteratingCallback return Action.SCHEDULED; } + private void onGenerateFailure(Throwable cause) + { + if (LOG.isDebugEnabled()) + LOG.debug("failed to generate {} on {}", entry, this, cause); + + accumulator.release(); + + entry.callback.failed(cause); + entry = null; + + // Continue the iteration. + succeeded(); + } + @Override protected void onSuccess() { @@ -96,8 +110,11 @@ public class MessageFlusher extends IteratingCallback accumulator.release(); - entry.callback.succeeded(); - entry = null; + if (entry != null) + { + entry.callback.succeeded(); + entry = null; + } } @Override @@ -108,11 +125,11 @@ public class MessageFlusher extends IteratingCallback accumulator.release(); - entry.callback.failed(cause); - entry = null; - - // Continue the iteration. - super.succeeded(); + if (entry != null) + { + entry.callback.failed(cause); + entry = null; + } } @Override diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java index 76d3bff375c..6fc58187dbb 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java @@ -356,7 +356,7 @@ public abstract class IteratingCallback implements Callback * to call {@code super.succeeded()}. */ @Override - public void succeeded() + public final void succeeded() { boolean process = false; try (AutoLock ignored = _lock.lock()) @@ -409,7 +409,7 @@ public abstract class IteratingCallback implements Callback * @see #isFailed() */ @Override - public void failed(Throwable x) + public final void failed(Throwable x) { boolean failure = false; try (AutoLock ignored = _lock.lock()) From d3c93bde273b0c5b501498dc11c62024a544f0de Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Wed, 26 Jun 2024 09:36:58 +0200 Subject: [PATCH 3/9] #11932 simplify queue handling Signed-off-by: Ludovic Orban --- .../jetty/quic/common/QuicConnection.java | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/jetty-core/jetty-quic/jetty-quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicConnection.java b/jetty-core/jetty-quic/jetty-quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicConnection.java index 722d494dbea..e5c1d340b65 100644 --- a/jetty-core/jetty-quic/jetty-quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicConnection.java +++ b/jetty-core/jetty-quic/jetty-quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicConnection.java @@ -17,11 +17,12 @@ import java.io.IOException; import java.net.InetSocketAddress; import java.net.SocketAddress; import java.nio.ByteBuffer; -import java.util.ArrayDeque; import java.util.Collection; import java.util.EventListener; import java.util.List; +import java.util.Queue; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Executor; @@ -39,7 +40,6 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IteratingCallback; import org.eclipse.jetty.util.component.LifeCycle; -import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.ExecutionStrategy; import org.eclipse.jetty.util.thread.Scheduler; import org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy; @@ -344,26 +344,19 @@ public abstract class QuicConnection extends AbstractConnection private class Flusher extends IteratingCallback { - private final AutoLock lock = new AutoLock(); - private final ArrayDeque queue = new ArrayDeque<>(); + private final Queue queue = new ConcurrentLinkedQueue<>(); private Entry entry; public void offer(Callback callback, SocketAddress address, ByteBuffer[] buffers) { - try (AutoLock l = lock.lock()) - { - queue.offer(new Entry(callback, address, buffers)); - } + queue.offer(new Entry(callback, address, buffers)); iterate(); } @Override protected Action process() { - try (AutoLock l = lock.lock()) - { - entry = queue.poll(); - } + entry = queue.poll(); if (entry == null) return Action.IDLE; From cea6b4ad98ca66df2bc6d837e6fcfaa3258357dd Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Wed, 26 Jun 2024 09:58:07 +0200 Subject: [PATCH 4/9] #11932 improve javadoc Signed-off-by: Ludovic Orban --- .../eclipse/jetty/util/IteratingCallback.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java index 6fc58187dbb..fb3b1ae7e42 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java @@ -168,7 +168,12 @@ public abstract class IteratingCallback implements Callback protected abstract Action process() throws Throwable; /** - * Invoked when one task has completed successfully. + * Invoked when one task has completed successfully, either by the + * caller thread or by the processing thread. This invocation is + * always serialized w.r.t the execution of {@link #process()}. + *

+ * This method is not invoked when a call to {@link #abort(Throwable)} + * is made before the {@link #succeeded()} callback happens. */ protected void onSuccess() { @@ -352,8 +357,11 @@ public abstract class IteratingCallback implements Callback /** * Method to invoke when the asynchronous sub-task succeeds. *

- * Subclasses that override this method must always remember - * to call {@code super.succeeded()}. + * This method should be considered final for all practical purposes. + *

+ * Eventually, {@link #onSuccess()} is + * called, either by the caller thread or by the processing + * thread. */ @Override public final void succeeded() @@ -399,8 +407,7 @@ public abstract class IteratingCallback implements Callback * or to fail the overall asynchronous task and therefore * terminate the iteration. *

- * Subclasses that override this method must always remember - * to call {@code super.failed(Throwable)}. + * This method should be considered final for all practical purposes. *

* Eventually, {@link #onCompleteFailure(Throwable)} is * called, either by the caller thread or by the processing From 4aaada09102428372d282936e12ae353bf54c24b Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Thu, 27 Jun 2024 10:57:47 +1000 Subject: [PATCH 5/9] force jetty-client in invoker it tests Signed-off-by: Olivier Lamy --- jetty-ee10/jetty-ee10-runner/pom.xml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/jetty-ee10/jetty-ee10-runner/pom.xml b/jetty-ee10/jetty-ee10-runner/pom.xml index 536670ec6a4..760e790dc42 100644 --- a/jetty-ee10/jetty-ee10-runner/pom.xml +++ b/jetty-ee10/jetty-ee10-runner/pom.xml @@ -131,6 +131,9 @@ ${it.debug} true + + org.eclipse.jetty:jetty-client:${project.version} + org.eclipse.jetty.maven.its.ee10.runner ${maven.dependency.plugin.version} From ddfc47c122a990bb58f8e9952db54eb1b1012c04 Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Thu, 27 Jun 2024 12:01:27 +1000 Subject: [PATCH 6/9] force jetty-client in invoker it tests Signed-off-by: Olivier Lamy --- jetty-ee9/jetty-ee9-runner/pom.xml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/jetty-ee9/jetty-ee9-runner/pom.xml b/jetty-ee9/jetty-ee9-runner/pom.xml index acaa2b545a2..3b4a8d0db72 100644 --- a/jetty-ee9/jetty-ee9-runner/pom.xml +++ b/jetty-ee9/jetty-ee9-runner/pom.xml @@ -133,6 +133,9 @@ ${it.debug} true + + org.eclipse.jetty:jetty-client:${project.version} + org.eclipse.jetty.maven.its.ee9.runner ${maven.dependency.plugin.version} From bae2e8f9bc17e222bcf8da5ff8c3da2c14a4f68c Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Thu, 27 Jun 2024 12:02:28 +1000 Subject: [PATCH 7/9] force jetty-client in invoker it tests Signed-off-by: Olivier Lamy --- jetty-ee8/jetty-ee8-runner/pom.xml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/jetty-ee8/jetty-ee8-runner/pom.xml b/jetty-ee8/jetty-ee8-runner/pom.xml index b07af47d1b4..3f420a1bb1a 100644 --- a/jetty-ee8/jetty-ee8-runner/pom.xml +++ b/jetty-ee8/jetty-ee8-runner/pom.xml @@ -135,6 +135,9 @@ ${it.debug} true + + org.eclipse.jetty:jetty-client:${project.version} + org.eclipse.jetty.maven.its.ee8.runner ${maven.dependency.plugin.version} From 6188757500f2e27bb03c70b27c2d713e5ae90613 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Thu, 27 Jun 2024 11:20:29 +0200 Subject: [PATCH 8/9] #11932 fix bug in case of invalid action + add test Signed-off-by: Ludovic Orban --- .../eclipse/jetty/util/IteratingCallback.java | 14 +++++------ .../jetty/util/IteratingCallbackTest.java | 25 +++++++++++++++++++ 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java index fb3b1ae7e42..5aba4b7a006 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java @@ -251,7 +251,6 @@ public abstract class IteratingCallback implements Callback boolean notifyCompleteSuccess = false; Throwable notifyCompleteFailure = null; - boolean callOnSuccess = false; // While we are processing processing: while (true) @@ -260,11 +259,6 @@ public abstract class IteratingCallback implements Callback Action action = null; try { - if (callOnSuccess) - { - onSuccess(); - callOnSuccess = false; - } action = process(); } catch (Throwable x) @@ -273,6 +267,7 @@ public abstract class IteratingCallback implements Callback // Fall through to possibly invoke onCompleteFailure(). } + boolean callOnSuccess = false; // acted on the action we have just received try (AutoLock ignored = _lock.lock()) { @@ -323,11 +318,11 @@ public abstract class IteratingCallback implements Callback case CALLED: { + callOnSuccess = true; if (action != Action.SCHEDULED) throw new IllegalStateException(String.format("%s[action=%s]", this, action)); // we lost the race, so we have to keep processing _state = State.PROCESSING; - callOnSuccess = true; continue; } @@ -346,6 +341,11 @@ public abstract class IteratingCallback implements Callback throw new IllegalStateException(String.format("%s[action=%s]", this, action)); } } + finally + { + if (callOnSuccess) + onSuccess(); + } } if (notifyCompleteSuccess) diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/IteratingCallbackTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/IteratingCallbackTest.java index b1e738a27dd..00f6b007b7d 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/IteratingCallbackTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/IteratingCallbackTest.java @@ -25,6 +25,7 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; public class IteratingCallbackTest @@ -435,4 +436,28 @@ public class IteratingCallbackTest assertEquals(1, count.get()); } + + @Test + public void testOnSuccessCalledDespiteISE() throws Exception + { + CountDownLatch latch = new CountDownLatch(1); + IteratingCallback icb = new IteratingCallback() + { + @Override + protected Action process() + { + succeeded(); + return Action.IDLE; // illegal action + } + + @Override + protected void onSuccess() + { + latch.countDown(); + } + }; + + assertThrows(IllegalStateException.class, icb::iterate); + assertTrue(latch.await(5, TimeUnit.SECONDS)); + } } From c0af0e6d8d3fe9b1c96ae2822d05e76508aaa409 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Thu, 27 Jun 2024 11:01:14 +0200 Subject: [PATCH 9/9] #11932 revert final modifier Signed-off-by: Ludovic Orban --- .../main/java/org/eclipse/jetty/util/IteratingCallback.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java index 5aba4b7a006..155577f58e2 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java @@ -364,7 +364,7 @@ public abstract class IteratingCallback implements Callback * thread. */ @Override - public final void succeeded() + public void succeeded() { boolean process = false; try (AutoLock ignored = _lock.lock()) @@ -416,7 +416,7 @@ public abstract class IteratingCallback implements Callback * @see #isFailed() */ @Override - public final void failed(Throwable x) + public void failed(Throwable x) { boolean failure = false; try (AutoLock ignored = _lock.lock())