From 7ee5cfc7e850667fc6a02e1805a4f5a4ebfe62f7 Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 27 Jun 2024 09:47:28 +1000 Subject: [PATCH 01/13] Fix flaky ContextScopeListenerTest --- .../servlet/ContextScopeListenerTest.java | 97 +++++++++++++++++-- .../ee9/servlet/ContextScopeListenerTest.java | 24 +++-- 2 files changed, 103 insertions(+), 18 deletions(-) diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ContextScopeListenerTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ContextScopeListenerTest.java index b35fa93c1a3..bdd08253fdf 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ContextScopeListenerTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ContextScopeListenerTest.java @@ -17,8 +17,8 @@ import java.net.URI; import java.util.Arrays; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; -import java.util.concurrent.locks.ReentrantLock; import jakarta.servlet.AsyncContext; import jakarta.servlet.DispatcherType; @@ -40,6 +40,7 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.junit.jupiter.api.Assertions.assertTrue; public class ContextScopeListenerTest { @@ -72,7 +73,7 @@ public class ContextScopeListenerTest } @Test - public void testAsyncServlet() throws Exception + public void testAsyncServletAsyncBeforeDoGetExit() throws Exception { _contextHandler.addServlet(new ServletHolder(new HttpServlet() { @@ -86,24 +87,33 @@ public class ContextScopeListenerTest } _history.add("doGet"); + CountDownLatch latch = new CountDownLatch(1); AsyncContext asyncContext = req.startAsync(); asyncContext.start(() -> { _history.add("asyncRunnable"); asyncContext.dispatch("/dispatch"); + latch.countDown(); + // wait until doGet call has exited + Awaitility.waitAtMost(5, TimeUnit.SECONDS).until(() -> _history.get(_history.size() - 1).equals("exitScope /initialPath")); }); + + try + { + assertTrue(latch.await(5, TimeUnit.SECONDS)); + } + catch (InterruptedException e) + { + throw new RuntimeException(e); + } } }), "/"); _contextHandler.addEventListener(new ContextHandler.ContextScopeListener() { - // Use a lock to prevent the async thread running the listener concurrently. - private final ReentrantLock _lock = new ReentrantLock(); - @Override public void enterScope(Context context, Request request) { - _lock.lock(); String pathInContext = (request == null) ? "null" : Request.getPathInContext(request); _history.add("enterScope " + pathInContext); } @@ -113,19 +123,90 @@ public class ContextScopeListenerTest { String pathInContext = (request == null) ? "null" : Request.getPathInContext(request); _history.add("exitScope " + pathInContext); - _lock.unlock(); } }); URI uri = URI.create("http://localhost:" + _connector.getLocalPort() + "/initialPath"); ContentResponse response = _client.GET(uri); assertThat(response.getStatus(), equalTo(HttpStatus.OK_200)); + Awaitility.waitAtMost(5, TimeUnit.SECONDS).pollInterval(100, TimeUnit.MILLISECONDS).until(() -> _history.size() == 7); + assertHistory( + "enterScope /initialPath", + "doGet", + "enterScope /initialPath", + "asyncRunnable", + "asyncDispatch", + "exitScope /initialPath", + "exitScope /initialPath" + ); + } + + @Test + public void testAsyncServletAsyncAfterDoGetExit() throws Exception + { + _contextHandler.addServlet(new ServletHolder(new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) + { + if (req.getDispatcherType() == DispatcherType.ASYNC) + { + _history.add("asyncDispatch"); + return; + } + + _history.add("doGet"); + CountDownLatch latch = new CountDownLatch(1); + AsyncContext asyncContext = req.startAsync(); + asyncContext.start(() -> + { + latch.countDown(); + // wait until doGet call has exited + Awaitility.waitAtMost(5, TimeUnit.SECONDS).until(() -> _history.get(_history.size() - 1).equals("exitScope /initialPath")); + + _history.add("asyncRunnable"); + asyncContext.dispatch("/dispatch"); + }); + + try + { + assertTrue(latch.await(5, TimeUnit.SECONDS)); + } + catch (InterruptedException e) + { + throw new RuntimeException(e); + } + + } + }), "/"); + + _contextHandler.addEventListener(new ContextHandler.ContextScopeListener() + { + @Override + public void enterScope(Context context, Request request) + { + String pathInContext = (request == null) ? "null" : Request.getPathInContext(request); + _history.add("enterScope " + pathInContext); + } + + @Override + public void exitScope(Context context, Request request) + { + String pathInContext = (request == null) ? "null" : Request.getPathInContext(request); + _history.add("exitScope " + pathInContext); + } + }); + + URI uri = URI.create("http://localhost:" + _connector.getLocalPort() + "/initialPath"); + ContentResponse response = _client.GET(uri); + assertThat(response.getStatus(), equalTo(HttpStatus.OK_200)); + Awaitility.waitAtMost(5, TimeUnit.SECONDS).pollInterval(100, TimeUnit.MILLISECONDS).until(() -> _history.size() == 9); assertHistory( "enterScope /initialPath", "doGet", - "exitScope /initialPath", "enterScope /initialPath", + "exitScope /initialPath", "asyncRunnable", "exitScope /initialPath", "enterScope /initialPath", diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ContextScopeListenerTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ContextScopeListenerTest.java index 3ae532c74bf..765995e980c 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ContextScopeListenerTest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ContextScopeListenerTest.java @@ -17,7 +17,8 @@ import java.net.URI; import java.util.Arrays; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.locks.ReentrantLock; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import jakarta.servlet.AsyncContext; import jakarta.servlet.DispatcherType; @@ -37,6 +38,7 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.junit.jupiter.api.Assertions.assertTrue; public class ContextScopeListenerTest { @@ -83,24 +85,31 @@ public class ContextScopeListenerTest } _history.add("doGet"); + CountDownLatch latch = new CountDownLatch(1); AsyncContext asyncContext = req.startAsync(); asyncContext.start(() -> { _history.add("asyncRunnable"); asyncContext.dispatch("/dispatch"); + latch.countDown(); }); + + try + { + assertTrue(latch.await(5, TimeUnit.SECONDS)); + } + catch (InterruptedException e) + { + throw new RuntimeException(e); + } } }), "/"); _contextHandler.addEventListener(new ContextHandler.ContextScopeListener() { - // Use a lock to prevent the async thread running the listener concurrently. - private final ReentrantLock _lock = new ReentrantLock(); - @Override public void enterScope(ContextHandler.APIContext context, org.eclipse.jetty.ee9.nested.Request request, Object reason) { - _lock.lock(); String pathInContext = (request == null) ? "null" : URIUtil.addPaths(request.getServletPath(), request.getPathInfo()); _history.add("enterScope " + pathInContext); } @@ -110,7 +119,6 @@ public class ContextScopeListenerTest { String pathInContext = (request == null) ? "null" : URIUtil.addPaths(request.getServletPath(), request.getPathInfo()); _history.add("exitScope " + pathInContext); - _lock.unlock(); } }); @@ -120,14 +128,10 @@ public class ContextScopeListenerTest assertHistory( "enterScope /initialPath", "doGet", - "exitScope /initialPath", "enterScope /initialPath", "asyncRunnable", "exitScope /initialPath", - "enterScope /dispatch", "asyncDispatch", - "exitScope /dispatch", - "enterScope /dispatch", "exitScope /dispatch" ); } From 6d0bddc07af28fde5513efe445f17281fb116661 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Mon, 24 Jun 2024 14:38:28 +0200 Subject: [PATCH 02/13] #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 03/13] #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 04/13] #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 05/13] #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 06/13] 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 07/13] 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 08/13] 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 09/13] #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 10/13] #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()) From 2caf9c11c09d5a6825dcc7ab85f0475843310543 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 26 Jun 2024 09:25:57 -0500 Subject: [PATCH 11/13] Cleanup XML environment specific deployment AppProviders --- VERSION.txt | 2 ++ .../src/main/config/etc/jetty-core-deploy.xml | 8 ++------ .../src/main/config/etc/jetty-ee10-quickstart.xml | 2 +- .../src/main/config/etc/jetty-ee10-deploy.xml | 9 +++------ .../src/main/config/etc/jetty-ee8-quickstart.xml | 2 +- .../src/main/config/etc/jetty-ee8-deploy.xml | 8 ++------ .../src/main/config/etc/jetty-ee9-quickstart.xml | 2 +- .../src/main/config/etc/jetty-ee9-deploy.xml | 9 +++------ 8 files changed, 15 insertions(+), 27 deletions(-) diff --git a/VERSION.txt b/VERSION.txt index 0c63546b804..259865a698a 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -414,6 +414,8 @@ jetty-10.0.18 - 26 October 2023 + 10537 HTTP/3: Incomplete Data Transfer When Used with Spring Boot WebFlux + 10696 jetty.sh doesn't work with JETTY_USER in Jetty 10.0.17 thru Jetty 12.0.2 + + 10669 Provide ability to defer initial deployment of webapps until after + Server has started + 10705 Creating a `HTTP3ServerConnector` with a `SslContextFactory` that has a non-null `SSLContext` makes the server fail to start with an unclear error message diff --git a/jetty-core/jetty-deploy/src/main/config/etc/jetty-core-deploy.xml b/jetty-core/jetty-deploy/src/main/config/etc/jetty-core-deploy.xml index bb55b9aa561..551fcc99b9a 100644 --- a/jetty-core/jetty-deploy/src/main/config/etc/jetty-core-deploy.xml +++ b/jetty-core/jetty-deploy/src/main/config/etc/jetty-core-deploy.xml @@ -1,11 +1,7 @@ - - - - - + @@ -20,7 +16,7 @@ - + core diff --git a/jetty-ee10/jetty-ee10-quickstart/src/main/config/etc/jetty-ee10-quickstart.xml b/jetty-ee10/jetty-ee10-quickstart/src/main/config/etc/jetty-ee10-quickstart.xml index 14788ab785c..a08f0c808c3 100644 --- a/jetty-ee10/jetty-ee10-quickstart/src/main/config/etc/jetty-ee10-quickstart.xml +++ b/jetty-ee10/jetty-ee10-quickstart/src/main/config/etc/jetty-ee10-quickstart.xml @@ -7,7 +7,7 @@ - + diff --git a/jetty-ee10/jetty-ee10-webapp/src/main/config/etc/jetty-ee10-deploy.xml b/jetty-ee10/jetty-ee10-webapp/src/main/config/etc/jetty-ee10-deploy.xml index 53872c8445b..868a0e51462 100644 --- a/jetty-ee10/jetty-ee10-webapp/src/main/config/etc/jetty-ee10-deploy.xml +++ b/jetty-ee10/jetty-ee10-webapp/src/main/config/etc/jetty-ee10-deploy.xml @@ -1,11 +1,7 @@ - - - - - + @@ -20,7 +16,7 @@ - + ee10 @@ -41,6 +37,7 @@ + diff --git a/jetty-ee8/jetty-ee8-quickstart/src/main/config/etc/jetty-ee8-quickstart.xml b/jetty-ee8/jetty-ee8-quickstart/src/main/config/etc/jetty-ee8-quickstart.xml index 1dd6e943f55..4a36d28e556 100644 --- a/jetty-ee8/jetty-ee8-quickstart/src/main/config/etc/jetty-ee8-quickstart.xml +++ b/jetty-ee8/jetty-ee8-quickstart/src/main/config/etc/jetty-ee8-quickstart.xml @@ -7,7 +7,7 @@ - + diff --git a/jetty-ee8/jetty-ee8-webapp/src/main/config/etc/jetty-ee8-deploy.xml b/jetty-ee8/jetty-ee8-webapp/src/main/config/etc/jetty-ee8-deploy.xml index 101c003028d..52fb1adae48 100644 --- a/jetty-ee8/jetty-ee8-webapp/src/main/config/etc/jetty-ee8-deploy.xml +++ b/jetty-ee8/jetty-ee8-webapp/src/main/config/etc/jetty-ee8-deploy.xml @@ -1,11 +1,7 @@ - - - - - + @@ -20,7 +16,7 @@ - + ee8 diff --git a/jetty-ee9/jetty-ee9-quickstart/src/main/config/etc/jetty-ee9-quickstart.xml b/jetty-ee9/jetty-ee9-quickstart/src/main/config/etc/jetty-ee9-quickstart.xml index 91606ec0ea5..e47a22440b1 100644 --- a/jetty-ee9/jetty-ee9-quickstart/src/main/config/etc/jetty-ee9-quickstart.xml +++ b/jetty-ee9/jetty-ee9-quickstart/src/main/config/etc/jetty-ee9-quickstart.xml @@ -7,7 +7,7 @@ - + diff --git a/jetty-ee9/jetty-ee9-webapp/src/main/config/etc/jetty-ee9-deploy.xml b/jetty-ee9/jetty-ee9-webapp/src/main/config/etc/jetty-ee9-deploy.xml index a0be5ab89f4..d4f60b484b1 100644 --- a/jetty-ee9/jetty-ee9-webapp/src/main/config/etc/jetty-ee9-deploy.xml +++ b/jetty-ee9/jetty-ee9-webapp/src/main/config/etc/jetty-ee9-deploy.xml @@ -1,11 +1,7 @@ - - - - - + @@ -20,7 +16,7 @@ - + ee9 @@ -41,6 +37,7 @@ + From 893911a8339007894f41ffc9fc8228fed9660a75 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Thu, 27 Jun 2024 18:50:03 +0200 Subject: [PATCH 12/13] Jetty 12.0.x 11774 backport env web xml (#11966) * Jetty 12.0.x jetty ee web xml (PR #11746) * Resolve jetty-eeX-web.xml before jetty-web.xml * Support jetty-eeX-env.xml, as per jetty-eeX-web.xml * Support jetty-eeX-env.xml, as per jetty-eeX-web.xml --------- Co-authored-by: Greg Wilkins --- .../ee10/plus/webapp/EnvConfiguration.java | 55 ++++++++---- .../plus/webapp/EnvConfigurationTest.java | 76 ++++++++++++++++ .../WEB-INF/jetty-ee10-env.xml | 15 ++++ .../WEB-INF/jetty-ee101-env.xml | 15 ++++ .../WEB-INF/jetty-env.xml | 15 ++++ .../WEB-INF/jetty-env.xml | 15 ++++ .../ee10/webapp/JettyWebXmlConfiguration.java | 88 ++++++++++++------- .../webapp/JettyWebXmlConfigurationTest.java | 73 +++++++++++++++ .../WEB-INF/jetty-ee10-web.xml | 19 ++++ .../WEB-INF/jetty-web.xml | 19 ++++ .../WEB-INF/jetty-web.xml | 19 ++++ .../WEB-INF/jetty-ee8-env.xml | 15 ++++ .../WEB-INF/jetty-ee88-env.xml | 15 ++++ .../WEB-INF/jetty-env.xml | 15 ++++ .../WEB-INF/jetty-env.xml | 15 ++++ .../WEB-INF/jetty-ee8-web.xml | 19 ++++ .../WEB-INF/jetty-web.xml | 19 ++++ .../ee9/plus/webapp/EnvConfiguration.java | 52 ++++++++--- .../ee9/plus/webapp/EnvConfigurationTest.java | 77 ++++++++++++++++ .../WEB-INF/jetty-ee9-env.xml | 15 ++++ .../WEB-INF/jetty-ee99-env.xml | 15 ++++ .../WEB-INF/jetty-env.xml | 15 ++++ .../WEB-INF/jetty-env.xml | 15 ++++ .../ee9/webapp/JettyWebXmlConfiguration.java | 88 ++++++++++++------- .../webapp/JettyWebXmlConfigurationTest.java | 73 +++++++++++++++ .../WEB-INF/jetty-ee9-web.xml | 19 ++++ .../WEB-INF/jetty-web.xml | 19 ++++ .../WEB-INF/jetty-web.xml | 19 ++++ 28 files changed, 824 insertions(+), 90 deletions(-) create mode 100644 jetty-ee10/jetty-ee10-plus/src/test/java/org/eclipse/jetty/ee10/plus/webapp/EnvConfigurationTest.java create mode 100644 jetty-ee10/jetty-ee10-plus/src/test/resources/webapp-with-jetty-ee10-env-xml/WEB-INF/jetty-ee10-env.xml create mode 100644 jetty-ee10/jetty-ee10-plus/src/test/resources/webapp-with-jetty-ee10-env-xml/WEB-INF/jetty-ee101-env.xml create mode 100644 jetty-ee10/jetty-ee10-plus/src/test/resources/webapp-with-jetty-ee10-env-xml/WEB-INF/jetty-env.xml create mode 100644 jetty-ee10/jetty-ee10-plus/src/test/resources/webapp-with-jetty-env-xml/WEB-INF/jetty-env.xml create mode 100644 jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/JettyWebXmlConfigurationTest.java create mode 100644 jetty-ee10/jetty-ee10-webapp/src/test/resources/webapp-with-jetty-ee10-web-xml/WEB-INF/jetty-ee10-web.xml create mode 100644 jetty-ee10/jetty-ee10-webapp/src/test/resources/webapp-with-jetty-ee10-web-xml/WEB-INF/jetty-web.xml create mode 100644 jetty-ee10/jetty-ee10-webapp/src/test/resources/webapp-with-jetty-web-xml/WEB-INF/jetty-web.xml create mode 100644 jetty-ee8/jetty-ee8-plus/src/test/resources/webapp-with-jetty-ee8-env-xml/WEB-INF/jetty-ee8-env.xml create mode 100644 jetty-ee8/jetty-ee8-plus/src/test/resources/webapp-with-jetty-ee8-env-xml/WEB-INF/jetty-ee88-env.xml create mode 100644 jetty-ee8/jetty-ee8-plus/src/test/resources/webapp-with-jetty-ee8-env-xml/WEB-INF/jetty-env.xml create mode 100644 jetty-ee8/jetty-ee8-plus/src/test/resources/webapp-with-jetty-env-xml/WEB-INF/jetty-env.xml create mode 100644 jetty-ee8/jetty-ee8-webapp/src/test/resources/webapp-with-jetty-ee8-web-xml/WEB-INF/jetty-ee8-web.xml create mode 100644 jetty-ee8/jetty-ee8-webapp/src/test/resources/webapp-with-jetty-ee8-web-xml/WEB-INF/jetty-web.xml create mode 100644 jetty-ee9/jetty-ee9-plus/src/test/java/org/eclipse/jetty/ee9/plus/webapp/EnvConfigurationTest.java create mode 100644 jetty-ee9/jetty-ee9-plus/src/test/resources/webapp-with-jetty-ee9-env-xml/WEB-INF/jetty-ee9-env.xml create mode 100644 jetty-ee9/jetty-ee9-plus/src/test/resources/webapp-with-jetty-ee9-env-xml/WEB-INF/jetty-ee99-env.xml create mode 100644 jetty-ee9/jetty-ee9-plus/src/test/resources/webapp-with-jetty-ee9-env-xml/WEB-INF/jetty-env.xml create mode 100644 jetty-ee9/jetty-ee9-plus/src/test/resources/webapp-with-jetty-env-xml/WEB-INF/jetty-env.xml create mode 100644 jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/JettyWebXmlConfigurationTest.java create mode 100644 jetty-ee9/jetty-ee9-webapp/src/test/resources/webapp-with-jetty-ee9-web-xml/WEB-INF/jetty-ee9-web.xml create mode 100644 jetty-ee9/jetty-ee9-webapp/src/test/resources/webapp-with-jetty-ee9-web-xml/WEB-INF/jetty-web.xml create mode 100644 jetty-ee9/jetty-ee9-webapp/src/test/resources/webapp-with-jetty-web-xml/WEB-INF/jetty-web.xml diff --git a/jetty-ee10/jetty-ee10-plus/src/main/java/org/eclipse/jetty/ee10/plus/webapp/EnvConfiguration.java b/jetty-ee10/jetty-ee10-plus/src/main/java/org/eclipse/jetty/ee10/plus/webapp/EnvConfiguration.java index 486ad1edd20..96b2e775fbb 100644 --- a/jetty-ee10/jetty-ee10-plus/src/main/java/org/eclipse/jetty/ee10/plus/webapp/EnvConfiguration.java +++ b/jetty-ee10/jetty-ee10-plus/src/main/java/org/eclipse/jetty/ee10/plus/webapp/EnvConfiguration.java @@ -47,6 +47,8 @@ public class EnvConfiguration extends AbstractConfiguration private static final Logger LOG = LoggerFactory.getLogger(EnvConfiguration.class); private static final String JETTY_ENV_BINDINGS = "org.eclipse.jetty.jndi.EnvConfiguration"; + private static final String JETTY_EE10_ENV_XML_FILENAME = "jetty-ee10-env.xml"; + private static final String JETTY_ENV_XML_FILENAME = "jetty-env.xml"; public EnvConfiguration() { @@ -69,25 +71,12 @@ public class EnvConfiguration extends AbstractConfiguration if (LOG.isDebugEnabled()) LOG.debug("Created java:comp/env for webapp {}", context.getContextPath()); - //check to see if an explicit file has been set, if not, - //look in WEB-INF/jetty-env.xml - + //check to see if an explicit file has been set Resource jettyEnvXmlResource = (Resource)context.getAttribute(JETTY_ENV_XML); if (jettyEnvXmlResource == null) { - //look for a file called WEB-INF/jetty-env.xml - //and process it if it exists - Resource webInf = context.getWebInf(); - if (webInf != null && webInf.isDirectory()) - { - // TODO: should never return from WEB-INF/lib/foo.jar!/WEB-INF/jetty-env.xml - // TODO: should also never return from a META-INF/versions/#/WEB-INF/jetty-env.xml location - Resource jettyEnv = webInf.resolve("jetty-env.xml"); - if (Resources.exists(jettyEnv)) - { - jettyEnvXmlResource = jettyEnv; - } - } + //otherwise find jetty-ee10-env.xml or fallback to jetty-env.xml + jettyEnvXmlResource = resolveJettyEnvXml(context.getWebInf()); } if (jettyEnvXmlResource != null) @@ -241,6 +230,40 @@ public class EnvConfiguration extends AbstractConfiguration } } + /** + * Obtain a WEB-INF/jetty-ee10-env.xml, falling back to + * looking for WEB-INF/jetty-env.xml. + * + * @param webInf the WEB-INF of the context to search + * @return the file if it exists or null otherwise + */ + private Resource resolveJettyEnvXml(Resource webInf) + { + try + { + if (webInf == null || !webInf.isDirectory()) + return null; + + //try to find jetty-ee10-env.xml + Resource xmlResource = webInf.resolve(JETTY_EE10_ENV_XML_FILENAME); + if (!Resources.missing(xmlResource)) + return xmlResource; + + //failing that, look for jetty-env.xml + xmlResource = webInf.resolve(JETTY_ENV_XML_FILENAME); + if (!Resources.missing(xmlResource)) + return xmlResource; + + return null; + } + catch (Exception e) + { + if (LOG.isDebugEnabled()) + LOG.debug("Error resolving", e); + return null; + } + } + private static class Dumper extends NamingDump { Dumper(ClassLoader loader, String name) diff --git a/jetty-ee10/jetty-ee10-plus/src/test/java/org/eclipse/jetty/ee10/plus/webapp/EnvConfigurationTest.java b/jetty-ee10/jetty-ee10-plus/src/test/java/org/eclipse/jetty/ee10/plus/webapp/EnvConfigurationTest.java new file mode 100644 index 00000000000..6dd104b62e2 --- /dev/null +++ b/jetty-ee10/jetty-ee10-plus/src/test/java/org/eclipse/jetty/ee10/plus/webapp/EnvConfigurationTest.java @@ -0,0 +1,76 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.ee10.plus.webapp; + +import java.nio.file.Files; +import java.nio.file.Path; + +import org.eclipse.jetty.ee10.webapp.WebAppContext; +import org.eclipse.jetty.plus.jndi.NamingEntryUtil; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.toolchain.test.MavenTestingUtils; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class EnvConfigurationTest +{ + Server _server; + + @BeforeEach + public void setUp() + { + _server = new Server(); + } + + @AfterEach + public void tearDown() throws Exception + { + if (_server != null) + _server.stop(); + } + + @Test + public void testWithOnlyJettyWebXml() throws Exception + { + Path testWebappDir = MavenTestingUtils.getTargetPath("test-classes/webapp-with-jetty-env-xml"); + assertTrue(Files.exists(testWebappDir)); + + WebAppContext context = new WebAppContext(); + context.setContextPath("/"); + _server.setHandler(context); + context.setWar(testWebappDir.toFile().getAbsolutePath()); + _server.start(); + assertNotNull(NamingEntryUtil.lookupNamingEntry(context, "apricot")); + } + + @Test + public void testWithJettyEEWebXml() throws Exception + { + Path testWebappDir = MavenTestingUtils.getTargetPath("test-classes/webapp-with-jetty-ee10-env-xml"); + assertTrue(Files.exists(testWebappDir)); + + WebAppContext context = new WebAppContext(); + context.setContextPath("/"); + _server.setHandler(context); + context.setWar(testWebappDir.toFile().getAbsolutePath()); + _server.start(); + assertNotNull(NamingEntryUtil.lookupNamingEntry(context, "peach")); + assertNull(NamingEntryUtil.lookupNamingEntry(context, "cabbage")); + } +} diff --git a/jetty-ee10/jetty-ee10-plus/src/test/resources/webapp-with-jetty-ee10-env-xml/WEB-INF/jetty-ee10-env.xml b/jetty-ee10/jetty-ee10-plus/src/test/resources/webapp-with-jetty-ee10-env-xml/WEB-INF/jetty-ee10-env.xml new file mode 100644 index 00000000000..5ad90b008ff --- /dev/null +++ b/jetty-ee10/jetty-ee10-plus/src/test/resources/webapp-with-jetty-ee10-env-xml/WEB-INF/jetty-ee10-env.xml @@ -0,0 +1,15 @@ + + + + + + + + + peach + 100 + true + + + + diff --git a/jetty-ee10/jetty-ee10-plus/src/test/resources/webapp-with-jetty-ee10-env-xml/WEB-INF/jetty-ee101-env.xml b/jetty-ee10/jetty-ee10-plus/src/test/resources/webapp-with-jetty-ee10-env-xml/WEB-INF/jetty-ee101-env.xml new file mode 100644 index 00000000000..c327a3a8123 --- /dev/null +++ b/jetty-ee10/jetty-ee10-plus/src/test/resources/webapp-with-jetty-ee10-env-xml/WEB-INF/jetty-ee101-env.xml @@ -0,0 +1,15 @@ + + + + + + + + + cabbage + 100 + true + + + + diff --git a/jetty-ee10/jetty-ee10-plus/src/test/resources/webapp-with-jetty-ee10-env-xml/WEB-INF/jetty-env.xml b/jetty-ee10/jetty-ee10-plus/src/test/resources/webapp-with-jetty-ee10-env-xml/WEB-INF/jetty-env.xml new file mode 100644 index 00000000000..fe3891b4686 --- /dev/null +++ b/jetty-ee10/jetty-ee10-plus/src/test/resources/webapp-with-jetty-ee10-env-xml/WEB-INF/jetty-env.xml @@ -0,0 +1,15 @@ + + + + + + + + + apricot + 100 + true + + + + diff --git a/jetty-ee10/jetty-ee10-plus/src/test/resources/webapp-with-jetty-env-xml/WEB-INF/jetty-env.xml b/jetty-ee10/jetty-ee10-plus/src/test/resources/webapp-with-jetty-env-xml/WEB-INF/jetty-env.xml new file mode 100644 index 00000000000..fe3891b4686 --- /dev/null +++ b/jetty-ee10/jetty-ee10-plus/src/test/resources/webapp-with-jetty-env-xml/WEB-INF/jetty-env.xml @@ -0,0 +1,15 @@ + + + + + + + + + apricot + 100 + true + + + + diff --git a/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/JettyWebXmlConfiguration.java b/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/JettyWebXmlConfiguration.java index 6eac106ff8c..9558e31394e 100644 --- a/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/JettyWebXmlConfiguration.java +++ b/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/JettyWebXmlConfiguration.java @@ -35,6 +35,7 @@ public class JettyWebXmlConfiguration extends AbstractConfiguration public static final String PROPERTY_WEB_INF = "web-inf"; public static final String XML_CONFIGURATION = "org.eclipse.jetty.webapp.JettyWebXmlConfiguration"; public static final String JETTY_WEB_XML = "jetty-web.xml"; + public static final String JETTY_EE10_WEB_XML = "jetty-ee10-web.xml"; public JettyWebXmlConfiguration() { @@ -55,41 +56,68 @@ public class JettyWebXmlConfiguration extends AbstractConfiguration LOG.debug("Configuring web-jetty.xml"); Resource webInf = context.getWebInf(); - // handle any WEB-INF descriptors - if (webInf != null && webInf.isDirectory()) + // get the jetty-ee10-web.xml or jetty-web.xml + Resource jetty = resolveJettyWebXml(webInf); + if (Resources.isReadableFile(jetty)) { - // Attempt to load ancient jetty8-web.xml file - Resource jetty = webInf.resolve("jetty8-web.xml"); - if (Resources.missing(jetty)) - jetty = webInf.resolve(JETTY_WEB_XML); - if (Resources.missing(jetty)) - jetty = webInf.resolve("web-jetty.xml"); + if (LOG.isDebugEnabled()) + LOG.debug("Configure: {}", jetty); - if (Resources.isReadableFile(jetty)) + Object xmlAttr = context.getAttribute(XML_CONFIGURATION); + context.removeAttribute(XML_CONFIGURATION); + final XmlConfiguration jetty_config = xmlAttr instanceof XmlConfiguration ? (XmlConfiguration)xmlAttr : new XmlConfiguration(jetty); + + setupXmlConfiguration(context, jetty_config, webInf); + + try { - if (LOG.isDebugEnabled()) - LOG.debug("Configure: {}", jetty); - - Object xmlAttr = context.getAttribute(XML_CONFIGURATION); - context.removeAttribute(XML_CONFIGURATION); - final XmlConfiguration jetty_config = xmlAttr instanceof XmlConfiguration ? (XmlConfiguration)xmlAttr : new XmlConfiguration(jetty); - - setupXmlConfiguration(context, jetty_config, webInf); - - try + WebAppClassLoader.runWithServerClassAccess(() -> { - WebAppClassLoader.runWithServerClassAccess(() -> - { - jetty_config.configure(context); - return null; - }); - } - catch (Exception e) - { - LOG.warn("Error applying {}", jetty); - throw e; - } + jetty_config.configure(context); + return null; + }); } + catch (Exception e) + { + LOG.warn("Error applying {}", jetty); + throw e; + } + } + } + + /** + * Obtain a WEB-INF/jetty-ee9-web.xml, falling back to + * looking for WEB-INF/jetty-web.xml. + * + * @param webInf the WEB-INF of the context to search + * @return the file if it exists or null otherwise + */ + private Resource resolveJettyWebXml(Resource webInf) + { + String xmlFile = JETTY_EE10_WEB_XML; + try + { + if (webInf == null || !webInf.isDirectory()) + return null; + + //try to find jetty-ee10-web.xml + Resource jetty = webInf.resolve(xmlFile); + if (!Resources.missing(jetty)) + return jetty; + + xmlFile = JETTY_WEB_XML; + //failing that, look for jetty-web.xml + jetty = webInf.resolve(xmlFile); + if (!Resources.missing(jetty)) + return jetty; + + return null; + } + catch (Exception e) + { + if (LOG.isDebugEnabled()) + LOG.debug("Error resolving WEB-INF/" + xmlFile, e); + return null; } } diff --git a/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/JettyWebXmlConfigurationTest.java b/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/JettyWebXmlConfigurationTest.java new file mode 100644 index 00000000000..84591fe0fe8 --- /dev/null +++ b/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/JettyWebXmlConfigurationTest.java @@ -0,0 +1,73 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.ee10.webapp; + +import java.nio.file.Files; +import java.nio.file.Path; + +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.toolchain.test.MavenTestingUtils; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class JettyWebXmlConfigurationTest +{ + Server _server; + + @BeforeEach + public void setUp() + { + _server = new Server(); + } + + @AfterEach + public void tearDown() throws Exception + { + if (_server != null) + _server.stop(); + } + + @Test + public void testWithOnlyJettyWebXml() throws Exception + { + Path testWebappDir = MavenTestingUtils.getTargetPath("test-classes/webapp-with-jetty-web-xml"); + assertTrue(Files.exists(testWebappDir)); + + WebAppContext context = new WebAppContext(); + context.setContextPath("/banana"); + _server.setHandler(context); + context.setWar(testWebappDir.toFile().getAbsolutePath()); + _server.start(); + assertThat(context.getContextPath(), is("/orange")); + } + + @Test + public void testWithJettyEEWebXml() throws Exception + { + Path testWebappDir = MavenTestingUtils.getTargetPath("test-classes/webapp-with-jetty-ee10-web-xml"); + assertTrue(Files.exists(testWebappDir)); + + WebAppContext context = new WebAppContext(); + context.setContextPath("/banana"); + _server.setHandler(context); + context.setWar(testWebappDir.toFile().getAbsolutePath()); + _server.start(); + assertThat(context.getContextPath(), is("/raspberry")); + } +} diff --git a/jetty-ee10/jetty-ee10-webapp/src/test/resources/webapp-with-jetty-ee10-web-xml/WEB-INF/jetty-ee10-web.xml b/jetty-ee10/jetty-ee10-webapp/src/test/resources/webapp-with-jetty-ee10-web-xml/WEB-INF/jetty-ee10-web.xml new file mode 100644 index 00000000000..beba698c2c9 --- /dev/null +++ b/jetty-ee10/jetty-ee10-webapp/src/test/resources/webapp-with-jetty-ee10-web-xml/WEB-INF/jetty-ee10-web.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + /raspberry + + diff --git a/jetty-ee10/jetty-ee10-webapp/src/test/resources/webapp-with-jetty-ee10-web-xml/WEB-INF/jetty-web.xml b/jetty-ee10/jetty-ee10-webapp/src/test/resources/webapp-with-jetty-ee10-web-xml/WEB-INF/jetty-web.xml new file mode 100644 index 00000000000..bc6363a9b73 --- /dev/null +++ b/jetty-ee10/jetty-ee10-webapp/src/test/resources/webapp-with-jetty-ee10-web-xml/WEB-INF/jetty-web.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + /apple + + diff --git a/jetty-ee10/jetty-ee10-webapp/src/test/resources/webapp-with-jetty-web-xml/WEB-INF/jetty-web.xml b/jetty-ee10/jetty-ee10-webapp/src/test/resources/webapp-with-jetty-web-xml/WEB-INF/jetty-web.xml new file mode 100644 index 00000000000..c1fd44b483b --- /dev/null +++ b/jetty-ee10/jetty-ee10-webapp/src/test/resources/webapp-with-jetty-web-xml/WEB-INF/jetty-web.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + /orange + + diff --git a/jetty-ee8/jetty-ee8-plus/src/test/resources/webapp-with-jetty-ee8-env-xml/WEB-INF/jetty-ee8-env.xml b/jetty-ee8/jetty-ee8-plus/src/test/resources/webapp-with-jetty-ee8-env-xml/WEB-INF/jetty-ee8-env.xml new file mode 100644 index 00000000000..d83bd1233a0 --- /dev/null +++ b/jetty-ee8/jetty-ee8-plus/src/test/resources/webapp-with-jetty-ee8-env-xml/WEB-INF/jetty-ee8-env.xml @@ -0,0 +1,15 @@ + + + + + + + + + peach + 100 + true + + + + diff --git a/jetty-ee8/jetty-ee8-plus/src/test/resources/webapp-with-jetty-ee8-env-xml/WEB-INF/jetty-ee88-env.xml b/jetty-ee8/jetty-ee8-plus/src/test/resources/webapp-with-jetty-ee8-env-xml/WEB-INF/jetty-ee88-env.xml new file mode 100644 index 00000000000..3f397d2244b --- /dev/null +++ b/jetty-ee8/jetty-ee8-plus/src/test/resources/webapp-with-jetty-ee8-env-xml/WEB-INF/jetty-ee88-env.xml @@ -0,0 +1,15 @@ + + + + + + + + + cabbage + 100 + true + + + + diff --git a/jetty-ee8/jetty-ee8-plus/src/test/resources/webapp-with-jetty-ee8-env-xml/WEB-INF/jetty-env.xml b/jetty-ee8/jetty-ee8-plus/src/test/resources/webapp-with-jetty-ee8-env-xml/WEB-INF/jetty-env.xml new file mode 100644 index 00000000000..01a352972a1 --- /dev/null +++ b/jetty-ee8/jetty-ee8-plus/src/test/resources/webapp-with-jetty-ee8-env-xml/WEB-INF/jetty-env.xml @@ -0,0 +1,15 @@ + + + + + + + + + apricot + 100 + true + + + + diff --git a/jetty-ee8/jetty-ee8-plus/src/test/resources/webapp-with-jetty-env-xml/WEB-INF/jetty-env.xml b/jetty-ee8/jetty-ee8-plus/src/test/resources/webapp-with-jetty-env-xml/WEB-INF/jetty-env.xml new file mode 100644 index 00000000000..01a352972a1 --- /dev/null +++ b/jetty-ee8/jetty-ee8-plus/src/test/resources/webapp-with-jetty-env-xml/WEB-INF/jetty-env.xml @@ -0,0 +1,15 @@ + + + + + + + + + apricot + 100 + true + + + + diff --git a/jetty-ee8/jetty-ee8-webapp/src/test/resources/webapp-with-jetty-ee8-web-xml/WEB-INF/jetty-ee8-web.xml b/jetty-ee8/jetty-ee8-webapp/src/test/resources/webapp-with-jetty-ee8-web-xml/WEB-INF/jetty-ee8-web.xml new file mode 100644 index 00000000000..75eb7ab66d9 --- /dev/null +++ b/jetty-ee8/jetty-ee8-webapp/src/test/resources/webapp-with-jetty-ee8-web-xml/WEB-INF/jetty-ee8-web.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + /raspberry + + diff --git a/jetty-ee8/jetty-ee8-webapp/src/test/resources/webapp-with-jetty-ee8-web-xml/WEB-INF/jetty-web.xml b/jetty-ee8/jetty-ee8-webapp/src/test/resources/webapp-with-jetty-ee8-web-xml/WEB-INF/jetty-web.xml new file mode 100644 index 00000000000..4cebf8aca2f --- /dev/null +++ b/jetty-ee8/jetty-ee8-webapp/src/test/resources/webapp-with-jetty-ee8-web-xml/WEB-INF/jetty-web.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + /apple + + diff --git a/jetty-ee9/jetty-ee9-plus/src/main/java/org/eclipse/jetty/ee9/plus/webapp/EnvConfiguration.java b/jetty-ee9/jetty-ee9-plus/src/main/java/org/eclipse/jetty/ee9/plus/webapp/EnvConfiguration.java index 7957901e970..7a87152e7a3 100644 --- a/jetty-ee9/jetty-ee9-plus/src/main/java/org/eclipse/jetty/ee9/plus/webapp/EnvConfiguration.java +++ b/jetty-ee9/jetty-ee9-plus/src/main/java/org/eclipse/jetty/ee9/plus/webapp/EnvConfiguration.java @@ -14,7 +14,6 @@ package org.eclipse.jetty.ee9.plus.webapp; import java.net.URL; -import java.util.Map; import java.util.Set; import javax.naming.Context; import javax.naming.InitialContext; @@ -50,6 +49,8 @@ public class EnvConfiguration extends AbstractConfiguration private static final Logger LOG = LoggerFactory.getLogger(EnvConfiguration.class); private static final String JETTY_ENV_BINDINGS = "org.eclipse.jetty.jndi.EnvConfiguration"; + private static final String JETTY_ENV_XML = "jetty-env.xml"; + private static final String JETTY_EE_ENV_XML = "jetty-ee9-env.xml"; private Resource jettyEnvXmlResource; private NamingDump _dumper; private ResourceFactory.Closeable _resourceFactory; @@ -89,19 +90,8 @@ public class EnvConfiguration extends AbstractConfiguration //look in WEB-INF/jetty-env.xml if (jettyEnvXmlResource == null) { - //look for a file called WEB-INF/jetty-env.xml - //and process it if it exists - org.eclipse.jetty.util.resource.Resource webInf = context.getWebInf(); - if (webInf != null && webInf.isDirectory()) - { - // TODO: should never return from WEB-INF/lib/foo.jar!/WEB-INF/jetty-env.xml - // TODO: should also never return from a META-INF/versions/#/WEB-INF/jetty-env.xml location - org.eclipse.jetty.util.resource.Resource jettyEnv = webInf.resolve("jetty-env.xml"); - if (Resources.isReadableFile(jettyEnv)) - { - jettyEnvXmlResource = jettyEnv; - } - } + //look for a configuration file + jettyEnvXmlResource = resolveJettyEnvXml(context.getWebInf()); } if (jettyEnvXmlResource != null) @@ -257,4 +247,38 @@ public class EnvConfiguration extends AbstractConfiguration Thread.currentThread().setContextClassLoader(oldLoader); } } + + /** + * Obtain a WEB-INF/jetty-ee9-env.xml, falling back to + * looking for WEB-INF/jetty-env.xml. + * + * @param webInf the WEB-INF of the context to search + * @return the file if it exists or null otherwise + */ + private Resource resolveJettyEnvXml(Resource webInf) + { + try + { + if (webInf == null || !webInf.isDirectory()) + return null; + + //try to find jetty-ee9-env.xml + Resource xmlResource = webInf.resolve(JETTY_EE_ENV_XML); + if (!Resources.missing(xmlResource)) + return xmlResource; + + //failing that, look for jetty-env.xml + xmlResource = webInf.resolve(JETTY_ENV_XML); + if (!Resources.missing(xmlResource)) + return xmlResource; + + return null; + } + catch (Exception e) + { + if (LOG.isDebugEnabled()) + LOG.debug("Error resolving", e); + return null; + } + } } diff --git a/jetty-ee9/jetty-ee9-plus/src/test/java/org/eclipse/jetty/ee9/plus/webapp/EnvConfigurationTest.java b/jetty-ee9/jetty-ee9-plus/src/test/java/org/eclipse/jetty/ee9/plus/webapp/EnvConfigurationTest.java new file mode 100644 index 00000000000..c7feca819e8 --- /dev/null +++ b/jetty-ee9/jetty-ee9-plus/src/test/java/org/eclipse/jetty/ee9/plus/webapp/EnvConfigurationTest.java @@ -0,0 +1,77 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.ee9.plus.webapp; + +import java.nio.file.Files; +import java.nio.file.Path; + +import org.eclipse.jetty.ee9.webapp.WebAppContext; +import org.eclipse.jetty.plus.jndi.NamingEntryUtil; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.toolchain.test.MavenTestingUtils; +import org.eclipse.jetty.util.jndi.NamingUtil; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class EnvConfigurationTest +{ + Server _server; + + @BeforeEach + public void setUp() + { + _server = new Server(); + } + + @AfterEach + public void tearDown() throws Exception + { + if (_server != null) + _server.stop(); + } + + @Test + public void testWithOnlyJettyWebXml() throws Exception + { + Path testWebappDir = MavenTestingUtils.getTargetPath("test-classes/webapp-with-jetty-env-xml"); + assertTrue(Files.exists(testWebappDir)); + + WebAppContext context = new WebAppContext(); + context.setContextPath("/"); + _server.setHandler(context); + context.setWar(testWebappDir.toFile().getAbsolutePath()); + _server.start(); + assertNotNull(NamingEntryUtil.lookupNamingEntry(context, "apricot")); + } + + @Test + public void testWithJettyEEWebXml() throws Exception + { + Path testWebappDir = MavenTestingUtils.getTargetPath("test-classes/webapp-with-jetty-ee9-env-xml"); + assertTrue(Files.exists(testWebappDir)); + + WebAppContext context = new WebAppContext(); + context.setContextPath("/"); + _server.setHandler(context); + context.setWar(testWebappDir.toFile().getAbsolutePath()); + _server.start(); + assertNotNull(NamingEntryUtil.lookupNamingEntry(context, "peach")); + assertNull(NamingEntryUtil.lookupNamingEntry(context, "cabbage")); + } +} diff --git a/jetty-ee9/jetty-ee9-plus/src/test/resources/webapp-with-jetty-ee9-env-xml/WEB-INF/jetty-ee9-env.xml b/jetty-ee9/jetty-ee9-plus/src/test/resources/webapp-with-jetty-ee9-env-xml/WEB-INF/jetty-ee9-env.xml new file mode 100644 index 00000000000..c0836565eb9 --- /dev/null +++ b/jetty-ee9/jetty-ee9-plus/src/test/resources/webapp-with-jetty-ee9-env-xml/WEB-INF/jetty-ee9-env.xml @@ -0,0 +1,15 @@ + + + + + + + + + peach + 100 + true + + + + diff --git a/jetty-ee9/jetty-ee9-plus/src/test/resources/webapp-with-jetty-ee9-env-xml/WEB-INF/jetty-ee99-env.xml b/jetty-ee9/jetty-ee9-plus/src/test/resources/webapp-with-jetty-ee9-env-xml/WEB-INF/jetty-ee99-env.xml new file mode 100644 index 00000000000..ee8b6917dbc --- /dev/null +++ b/jetty-ee9/jetty-ee9-plus/src/test/resources/webapp-with-jetty-ee9-env-xml/WEB-INF/jetty-ee99-env.xml @@ -0,0 +1,15 @@ + + + + + + + + + cabbage + 100 + true + + + + diff --git a/jetty-ee9/jetty-ee9-plus/src/test/resources/webapp-with-jetty-ee9-env-xml/WEB-INF/jetty-env.xml b/jetty-ee9/jetty-ee9-plus/src/test/resources/webapp-with-jetty-ee9-env-xml/WEB-INF/jetty-env.xml new file mode 100644 index 00000000000..0e0592571ce --- /dev/null +++ b/jetty-ee9/jetty-ee9-plus/src/test/resources/webapp-with-jetty-ee9-env-xml/WEB-INF/jetty-env.xml @@ -0,0 +1,15 @@ + + + + + + + + + apricot + 100 + true + + + + diff --git a/jetty-ee9/jetty-ee9-plus/src/test/resources/webapp-with-jetty-env-xml/WEB-INF/jetty-env.xml b/jetty-ee9/jetty-ee9-plus/src/test/resources/webapp-with-jetty-env-xml/WEB-INF/jetty-env.xml new file mode 100644 index 00000000000..0e0592571ce --- /dev/null +++ b/jetty-ee9/jetty-ee9-plus/src/test/resources/webapp-with-jetty-env-xml/WEB-INF/jetty-env.xml @@ -0,0 +1,15 @@ + + + + + + + + + apricot + 100 + true + + + + diff --git a/jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/JettyWebXmlConfiguration.java b/jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/JettyWebXmlConfiguration.java index 1c4728be8a1..e1f1a703b68 100644 --- a/jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/JettyWebXmlConfiguration.java +++ b/jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/JettyWebXmlConfiguration.java @@ -35,6 +35,7 @@ public class JettyWebXmlConfiguration extends AbstractConfiguration public static final String PROPERTY_WEB_INF = "web-inf"; public static final String XML_CONFIGURATION = "org.eclipse.jetty.ee9.webapp.JettyWebXmlConfiguration"; public static final String JETTY_WEB_XML = "jetty-web.xml"; + public static final String JETTY_EE_9_WEB_XML = "jetty-ee9-web.xml"; public JettyWebXmlConfiguration() { @@ -54,41 +55,68 @@ public class JettyWebXmlConfiguration extends AbstractConfiguration LOG.debug("Configuring web-jetty.xml"); Resource webInf = context.getWebInf(); - // handle any WEB-INF descriptors - if (webInf != null && webInf.isDirectory()) + // get the jetty-ee9-web.xml or jetty-web.xml + Resource jetty = resolveJettyWebXml(webInf); + if (Resources.isReadableFile(jetty)) { - // do jetty.xml file - Resource jetty = webInf.resolve("jetty8-web.xml"); - if (Resources.missing(jetty)) - jetty = webInf.resolve(JETTY_WEB_XML); - if (Resources.missing(jetty)) - jetty = webInf.resolve("web-jetty.xml"); + if (LOG.isDebugEnabled()) + LOG.debug("Configure: {}", jetty); - if (Resources.isReadableFile(jetty)) + Object xmlAttr = context.getAttribute(XML_CONFIGURATION); + context.removeAttribute(XML_CONFIGURATION); + final XmlConfiguration jetty_config = xmlAttr instanceof XmlConfiguration ? (XmlConfiguration)xmlAttr : new XmlConfiguration(jetty); + + setupXmlConfiguration(context, jetty_config, webInf); + + try { - if (LOG.isDebugEnabled()) - LOG.debug("Configure: {}", jetty); - - Object xmlAttr = context.getAttribute(XML_CONFIGURATION); - context.removeAttribute(XML_CONFIGURATION); - final XmlConfiguration jetty_config = xmlAttr instanceof XmlConfiguration ? (XmlConfiguration)xmlAttr : new XmlConfiguration(jetty); - - setupXmlConfiguration(context, jetty_config, webInf); - - try + WebAppClassLoader.runWithServerClassAccess(() -> { - WebAppClassLoader.runWithServerClassAccess(() -> - { - jetty_config.configure(context); - return null; - }); - } - catch (Exception e) - { - LOG.warn("Error applying {}", jetty); - throw e; - } + jetty_config.configure(context); + return null; + }); } + catch (Exception e) + { + LOG.warn("Error applying {}", jetty); + throw e; + } + } + } + + /** + * Obtain a WEB-INF/jetty-ee9-web.xml, falling back to + * looking for WEB-INF/jetty-web.xml. + * + * @param webInf the WEB-INF of the context to search + * @return the file if it exists or null otherwise + */ + private Resource resolveJettyWebXml(Resource webInf) + { + String xmlFile = JETTY_EE_9_WEB_XML; + try + { + if (webInf == null || !webInf.isDirectory()) + return null; + + //try to find jetty-ee9-web.xml + Resource jetty = webInf.resolve(xmlFile); + if (!Resources.missing(jetty)) + return jetty; + + xmlFile = JETTY_WEB_XML; + //failing that, look for jetty-web.xml + jetty = webInf.resolve(xmlFile); + if (!Resources.missing(jetty)) + return jetty; + + return null; + } + catch (Exception e) + { + if (LOG.isDebugEnabled()) + LOG.debug("Error resolving WEB-INF/" + xmlFile, e); + return null; } } diff --git a/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/JettyWebXmlConfigurationTest.java b/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/JettyWebXmlConfigurationTest.java new file mode 100644 index 00000000000..891cf0a75e5 --- /dev/null +++ b/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/JettyWebXmlConfigurationTest.java @@ -0,0 +1,73 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.ee9.webapp; + +import java.nio.file.Files; +import java.nio.file.Path; + +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.toolchain.test.MavenTestingUtils; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class JettyWebXmlConfigurationTest +{ + Server _server; + + @BeforeEach + public void setUp() + { + _server = new Server(); + } + + @AfterEach + public void tearDown() throws Exception + { + if (_server != null) + _server.stop(); + } + + @Test + public void testWithOnlyJettyWebXml() throws Exception + { + Path testWebappDir = MavenTestingUtils.getTargetPath("test-classes/webapp-with-jetty-web-xml"); + assertTrue(Files.exists(testWebappDir)); + + WebAppContext context = new WebAppContext(); + context.setContextPath("/banana"); + _server.setHandler(context); + context.setWar(testWebappDir.toFile().getAbsolutePath()); + _server.start(); + assertThat(context.getContextPath(), is("/orange")); + } + + @Test + public void testWithJettyEEWebXml() throws Exception + { + Path testWebappDir = MavenTestingUtils.getTargetPath("test-classes/webapp-with-jetty-ee9-web-xml"); + assertTrue(Files.exists(testWebappDir)); + + WebAppContext context = new WebAppContext(); + context.setContextPath("/banana"); + _server.setHandler(context); + context.setWar(testWebappDir.toFile().getAbsolutePath()); + _server.start(); + assertThat(context.getContextPath(), is("/raspberry")); + } +} diff --git a/jetty-ee9/jetty-ee9-webapp/src/test/resources/webapp-with-jetty-ee9-web-xml/WEB-INF/jetty-ee9-web.xml b/jetty-ee9/jetty-ee9-webapp/src/test/resources/webapp-with-jetty-ee9-web-xml/WEB-INF/jetty-ee9-web.xml new file mode 100644 index 00000000000..68227056378 --- /dev/null +++ b/jetty-ee9/jetty-ee9-webapp/src/test/resources/webapp-with-jetty-ee9-web-xml/WEB-INF/jetty-ee9-web.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + /raspberry + + diff --git a/jetty-ee9/jetty-ee9-webapp/src/test/resources/webapp-with-jetty-ee9-web-xml/WEB-INF/jetty-web.xml b/jetty-ee9/jetty-ee9-webapp/src/test/resources/webapp-with-jetty-ee9-web-xml/WEB-INF/jetty-web.xml new file mode 100644 index 00000000000..517bef72c45 --- /dev/null +++ b/jetty-ee9/jetty-ee9-webapp/src/test/resources/webapp-with-jetty-ee9-web-xml/WEB-INF/jetty-web.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + /apple + + diff --git a/jetty-ee9/jetty-ee9-webapp/src/test/resources/webapp-with-jetty-web-xml/WEB-INF/jetty-web.xml b/jetty-ee9/jetty-ee9-webapp/src/test/resources/webapp-with-jetty-web-xml/WEB-INF/jetty-web.xml new file mode 100644 index 00000000000..90d5ac4e64a --- /dev/null +++ b/jetty-ee9/jetty-ee9-webapp/src/test/resources/webapp-with-jetty-web-xml/WEB-INF/jetty-web.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + /orange + + From 5e8cc2243ebe54a113e529df76c980346d528ada Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 28 Jun 2024 09:37:09 +1000 Subject: [PATCH 13/13] Remove buffer from pool on write failure (#11951) * Experiment with removable buffer from pool * Changed remove return to be release boolean * Fixes to avoid double release * Tracking ByteBufferPool handles remove * Adding assert on _channelState.isLockHeldByCurrentThread() --------- Co-authored-by: Joakim Erdfelt --- .../eclipse/jetty/io/ArrayByteBufferPool.java | 113 +++++++++++++----- .../org/eclipse/jetty/io/ByteBufferPool.java | 14 +++ .../jetty/io/ArrayByteBufferPoolTest.java | 40 +++++++ .../jetty/server/handler/ErrorHandler.java | 39 ++++-- .../jetty/ee10/servlet/HttpOutput.java | 34 ++++-- .../ee10/servlet/ServletChannelState.java | 5 + .../jetty/ee9/nested/HttpChannelState.java | 5 + .../eclipse/jetty/ee9/nested/HttpOutput.java | 33 +++-- 8 files changed, 222 insertions(+), 61 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java index 38d3227e0ff..eff48e66683 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java @@ -20,13 +20,13 @@ import java.nio.ByteBuffer; import java.time.Instant; import java.util.Arrays; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.LongAdder; -import java.util.function.Consumer; import java.util.function.IntUnaryOperator; import java.util.stream.Collectors; @@ -205,24 +205,49 @@ public class ArrayByteBufferPool implements ByteBufferPool, Dumpable // No bucket, return non-pooled. if (bucket == null) - return newRetainableByteBuffer(size, direct, null); + return RetainableByteBuffer.wrap(BufferUtil.allocate(size, direct)); bucket.recordAcquire(); // Try to acquire a pooled entry. Pool.Entry entry = bucket.getPool().acquire(); - if (entry != null) + if (entry == null) { - bucket.recordPooled(); - RetainableByteBuffer buffer = entry.getPooled(); - ((Buffer)buffer).acquire(); - return buffer; + ByteBuffer buffer = BufferUtil.allocate(bucket.getCapacity(), direct); + return new ReservedBuffer(buffer, bucket); } - return newRetainableByteBuffer(bucket.getCapacity(), direct, buffer -> reserve(bucket, buffer)); + bucket.recordPooled(); + RetainableByteBuffer buffer = entry.getPooled(); + ((Buffer)buffer).acquire(); + return buffer; } - private void reserve(RetainedBucket bucket, RetainableByteBuffer buffer) + @Override + public boolean removeAndRelease(RetainableByteBuffer buffer) + { + RetainableByteBuffer actual = buffer; + while (actual instanceof RetainableByteBuffer.Wrapper wrapper) + actual = wrapper.getWrapped(); + + if (actual instanceof ReservedBuffer reservedBuffer) + { + // remove the actual reserved buffer, but release the wrapped buffer + reservedBuffer.remove(); + return buffer.release(); + } + + if (actual instanceof Buffer poolBuffer) + { + // remove the actual pool buffer, but release the wrapped buffer + poolBuffer.remove(); + return buffer.release(); + } + + return ByteBufferPool.super.removeAndRelease(buffer); + } + + private void reserve(RetainedBucket bucket, ByteBuffer byteBuffer) { bucket.recordRelease(); @@ -235,12 +260,11 @@ public class ArrayByteBufferPool implements ByteBufferPool, Dumpable } // Add the buffer to the new entry. - ByteBuffer byteBuffer = buffer.getByteBuffer(); BufferUtil.reset(byteBuffer); - Buffer pooledBuffer = new Buffer(byteBuffer, b -> release(bucket, entry)); + Buffer pooledBuffer = new Buffer(byteBuffer, bucket, entry); if (entry.enable(pooledBuffer, false)) { - checkMaxMemory(bucket, buffer.isDirect()); + checkMaxMemory(bucket, byteBuffer.isDirect()); return; } @@ -270,6 +294,13 @@ public class ArrayByteBufferPool implements ByteBufferPool, Dumpable entry.remove(); } + private boolean remove(RetainedBucket bucket, Pool.Entry entry) + { + // Cannot release, discard this buffer. + bucket.recordRemove(); + return entry.remove(); + } + private void checkMaxMemory(RetainedBucket bucket, boolean direct) { long max = direct ? _maxDirectMemory : _maxHeapMemory; @@ -309,14 +340,6 @@ public class ArrayByteBufferPool implements ByteBufferPool, Dumpable } } - private RetainableByteBuffer newRetainableByteBuffer(int capacity, boolean direct, Consumer releaser) - { - ByteBuffer buffer = BufferUtil.allocate(capacity, direct); - Buffer retainableByteBuffer = new Buffer(buffer, releaser); - retainableByteBuffer.acquire(); - return retainableByteBuffer; - } - public Pool poolFor(int capacity, boolean direct) { RetainedBucket bucket = bucketFor(capacity, direct); @@ -581,15 +604,45 @@ public class ArrayByteBufferPool implements ByteBufferPool, Dumpable } } - private static class Buffer extends AbstractRetainableByteBuffer + private class ReservedBuffer extends AbstractRetainableByteBuffer { - private final Consumer _releaser; - private int _usages; + private final RetainedBucket _bucket; + private final AtomicBoolean _removed = new AtomicBoolean(); - private Buffer(ByteBuffer buffer, Consumer releaser) + private ReservedBuffer(ByteBuffer buffer, RetainedBucket bucket) { super(buffer); - this._releaser = releaser; + _bucket = Objects.requireNonNull(bucket); + acquire(); + } + + @Override + public boolean release() + { + boolean released = super.release(); + if (released && _removed.compareAndSet(false, true)) + reserve(_bucket, getByteBuffer()); + return released; + } + + boolean remove() + { + // Buffer never added to pool, so just prevent future reservation + return _removed.compareAndSet(false, true); + } + } + + private class Buffer extends AbstractRetainableByteBuffer + { + private final RetainedBucket _bucket; + private final Pool.Entry _entry; + private int _usages; + + private Buffer(ByteBuffer buffer, RetainedBucket bucket, Pool.Entry entry) + { + super(buffer); + _bucket = Objects.requireNonNull(bucket); + _entry = Objects.requireNonNull(entry); } @Override @@ -597,13 +650,15 @@ public class ArrayByteBufferPool implements ByteBufferPool, Dumpable { boolean released = super.release(); if (released) - { - if (_releaser != null) - _releaser.accept(this); - } + ArrayByteBufferPool.this.release(_bucket, _entry); return released; } + boolean remove() + { + return ArrayByteBufferPool.this.remove(_bucket, _entry); + } + private int use() { if (++_usages < 0) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java index 2233ae72bf4..e988d28efbf 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java @@ -58,6 +58,20 @@ public interface ByteBufferPool */ RetainableByteBuffer acquire(int size, boolean direct); + /** + * {@link RetainableByteBuffer#release() Release} the buffer in a way that will remove it from any pool that it may be in. + * If the buffer is not in a pool, calling this method is equivalent to calling {@link RetainableByteBuffer#release()}. + * Calling this method satisfies any contract that requires a call to {@link RetainableByteBuffer#release()}. + * @return {@code true} if a call to {@link RetainableByteBuffer#release()} would have returned {@code true}. + * @see RetainableByteBuffer#release() + * @deprecated This API is experimental and may be removed in future releases + */ + @Deprecated + default boolean removeAndRelease(RetainableByteBuffer buffer) + { + return buffer != null && buffer.release(); + } + /** *

Removes all {@link RetainableByteBuffer#isRetained() non-retained} * pooled instances from this pool.

diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java index 59040fb2456..bb92c805080 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java @@ -32,6 +32,7 @@ import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.sameInstance; import static org.hamcrest.core.Is.is; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -444,4 +445,43 @@ public class ArrayByteBufferPoolTest assertThat(compoundPool.getPrimaryPool().size(), is(ConcurrentPool.OPTIMAL_MAX_SIZE)); assertThat(compoundPool.getSecondaryPool().size(), is(0)); } + + @Test + public void testRemoveAndRelease() + { + ArrayByteBufferPool pool = new ArrayByteBufferPool(); + + RetainableByteBuffer reserved0 = pool.acquire(1024, false); + RetainableByteBuffer reserved1 = pool.acquire(1024, false); + + RetainableByteBuffer acquired0 = pool.acquire(1024, false); + acquired0.release(); + acquired0 = pool.acquire(1024, false); + RetainableByteBuffer acquired1 = pool.acquire(1024, false); + acquired1.release(); + acquired1 = pool.acquire(1024, false); + + RetainableByteBuffer retained0 = pool.acquire(1024, false); + retained0.release(); + retained0 = pool.acquire(1024, false); + retained0.retain(); + RetainableByteBuffer retained1 = pool.acquire(1024, false); + retained1.release(); + retained1 = pool.acquire(1024, false); + retained1.retain(); + + assertTrue(pool.removeAndRelease(reserved1)); + assertTrue(pool.removeAndRelease(acquired1)); + assertFalse(pool.removeAndRelease(retained1)); + assertTrue(retained1.release()); + + assertThat(pool.getHeapByteBufferCount(), is(2L)); + assertTrue(reserved0.release()); + assertThat(pool.getHeapByteBufferCount(), is(3L)); + assertTrue(acquired0.release()); + assertThat(pool.getHeapByteBufferCount(), is(3L)); + assertFalse(retained0.release()); + assertTrue(retained0.release()); + assertThat(pool.getHeapByteBufferCount(), is(3L)); + } } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java index ee0432b3116..d32e4255c39 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java @@ -28,6 +28,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; import org.eclipse.jetty.http.HttpException; @@ -39,14 +40,15 @@ import org.eclipse.jetty.http.MimeTypes.Type; import org.eclipse.jetty.http.PreEncodedHttpField; import org.eclipse.jetty.http.QuotedQualityCSV; import org.eclipse.jetty.io.ByteBufferOutputStream; +import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.Content; -import org.eclipse.jetty.io.Retainable; import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.util.Attributes; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.ExceptionUtil; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; @@ -198,7 +200,8 @@ public class ErrorHandler implements Request.Handler int bufferSize = request.getConnectionMetaData().getHttpConfiguration().getOutputBufferSize(); bufferSize = Math.min(8192, bufferSize); // TODO ? - RetainableByteBuffer buffer = request.getComponents().getByteBufferPool().acquire(bufferSize, false); + ByteBufferPool byteBufferPool = request.getComponents().getByteBufferPool(); + RetainableByteBuffer buffer = byteBufferPool.acquire(bufferSize, false); try { @@ -251,13 +254,14 @@ public class ErrorHandler implements Request.Handler } response.getHeaders().put(type.getContentTypeField(charset)); - response.write(true, buffer.getByteBuffer(), new WriteErrorCallback(callback, buffer)); + response.write(true, buffer.getByteBuffer(), new WriteErrorCallback(callback, byteBufferPool, buffer)); return true; } catch (Throwable x) { - buffer.release(); + if (buffer != null) + byteBufferPool.removeAndRelease(buffer); throw x; } } @@ -579,20 +583,33 @@ public class ErrorHandler implements Request.Handler * when calling {@link Response#write(boolean, ByteBuffer, Callback)} to wrap the passed in {@link Callback} * so that the {@link RetainableByteBuffer} used can be released. */ - private static class WriteErrorCallback extends Callback.Nested + private static class WriteErrorCallback implements Callback { - private final Retainable _retainable; + private final AtomicReference _callback; + private final ByteBufferPool _pool; + private final RetainableByteBuffer _buffer; - public WriteErrorCallback(Callback callback, Retainable retainable) + public WriteErrorCallback(Callback callback, ByteBufferPool pool, RetainableByteBuffer retainable) { - super(callback); - _retainable = retainable; + _callback = new AtomicReference<>(callback); + _pool = pool; + _buffer = retainable; } @Override - public void completed() + public void succeeded() { - _retainable.release(); + Callback callback = _callback.getAndSet(null); + if (callback != null) + ExceptionUtil.callAndThen(_buffer::release, callback::succeeded); + } + + @Override + public void failed(Throwable x) + { + Callback callback = _callback.getAndSet(null); + if (callback != null) + ExceptionUtil.callAndThen(x, t -> _pool.removeAndRelease(_buffer), callback::failed); } } } diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpOutput.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpOutput.java index b9e6270401d..0ac43556411 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpOutput.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpOutput.java @@ -135,6 +135,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable private long _written; private long _flushed; private long _firstByteNanoTime = -1; + private ByteBufferPool _pool; private RetainableByteBuffer _aggregate; private int _bufferSize; private int _commitSize; @@ -222,7 +223,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable _state = State.CLOSED; closedCallback = _closedCallback; _closedCallback = null; - releaseBuffer(); + lockedReleaseBuffer(failure != null); wake = updateApiState(failure); } else if (_state == State.CLOSE) @@ -444,7 +445,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable try (AutoLock l = _channelState.lock()) { _state = State.CLOSED; - releaseBuffer(); + lockedReleaseBuffer(failure != null); } } @@ -576,25 +577,36 @@ public class HttpOutput extends ServletOutputStream implements Runnable { try (AutoLock l = _channelState.lock()) { - return acquireBuffer().getByteBuffer(); + return lockedAcquireBuffer().getByteBuffer(); } } - private RetainableByteBuffer acquireBuffer() + private RetainableByteBuffer lockedAcquireBuffer() { + assert _channelState.isLockHeldByCurrentThread(); + boolean useOutputDirectByteBuffers = _servletChannel.getConnectionMetaData().getHttpConfiguration().isUseOutputDirectByteBuffers(); - ByteBufferPool pool = _servletChannel.getRequest().getComponents().getByteBufferPool(); + if (_aggregate == null) - _aggregate = pool.acquire(getBufferSize(), useOutputDirectByteBuffers); + { + _pool = _servletChannel.getRequest().getComponents().getByteBufferPool(); + _aggregate = _pool.acquire(getBufferSize(), useOutputDirectByteBuffers); + } return _aggregate; } - private void releaseBuffer() + private void lockedReleaseBuffer(boolean failure) { + assert _channelState.isLockHeldByCurrentThread(); + if (_aggregate != null) { - _aggregate.release(); + if (failure && _pool != null) + _pool.removeAndRelease(_aggregate); + else + _aggregate.release(); _aggregate = null; + _pool = null; } } @@ -757,7 +769,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable // Should we aggregate? if (aggregate) { - acquireBuffer(); + lockedAcquireBuffer(); int filled = BufferUtil.fill(_aggregate.getByteBuffer(), b, off, len); // return if we are not complete, not full and filled all the content @@ -962,7 +974,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable } _written = written; - acquireBuffer(); + lockedAcquireBuffer(); BufferUtil.append(_aggregate.getByteBuffer(), (byte)b); } @@ -1265,6 +1277,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable { try (AutoLock l = _channelState.lock()) { + lockedReleaseBuffer(_state != State.CLOSED); _state = State.OPEN; _apiState = ApiState.BLOCKING; _softClose = true; // Stay closed until next request @@ -1273,7 +1286,6 @@ public class HttpOutput extends ServletOutputStream implements Runnable _commitSize = config.getOutputAggregationSize(); if (_commitSize > _bufferSize) _commitSize = _bufferSize; - releaseBuffer(); _written = 0; _writeListener = null; _onError = null; diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannelState.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannelState.java index 0c419cba559..da225815373 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannelState.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannelState.java @@ -227,6 +227,11 @@ public class ServletChannelState return _lock.lock(); } + boolean isLockHeldByCurrentThread() + { + return _lock.isHeldByCurrentThread(); + } + public State getState() { try (AutoLock ignored = lock()) diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpChannelState.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpChannelState.java index 7fdc4184b06..b05626a4424 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpChannelState.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpChannelState.java @@ -158,6 +158,11 @@ public class HttpChannelState return _lock.lock(); } + boolean isLockHeldByCurrentThread() + { + return _lock.isHeldByCurrentThread(); + } + public State getState() { try (AutoLock l = lock()) diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpOutput.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpOutput.java index b326486e504..c4600889f71 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpOutput.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpOutput.java @@ -32,6 +32,7 @@ import jakarta.servlet.ServletRequest; import jakarta.servlet.ServletResponse; import jakarta.servlet.WriteListener; import org.eclipse.jetty.http.content.HttpContent; +import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.io.IOResources; import org.eclipse.jetty.io.RetainableByteBuffer; @@ -191,6 +192,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable private long _written; private long _flushed; private long _firstByteNanoTime = -1; + private ByteBufferPool _pool; private RetainableByteBuffer _aggregate; private int _bufferSize; private int _commitSize; @@ -292,7 +294,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable _state = State.CLOSED; closedCallback = _closedCallback; _closedCallback = null; - releaseBuffer(); + lockedReleaseBuffer(failure != null); wake = updateApiState(failure); } else if (_state == State.CLOSE) @@ -513,7 +515,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable try (AutoLock l = _channelState.lock()) { _state = State.CLOSED; - releaseBuffer(); + lockedReleaseBuffer(failure != null); } } @@ -642,23 +644,34 @@ public class HttpOutput extends ServletOutputStream implements Runnable { try (AutoLock l = _channelState.lock()) { - return acquireBuffer().getByteBuffer(); + return lockedAcquireBuffer().getByteBuffer(); } } - private RetainableByteBuffer acquireBuffer() + private RetainableByteBuffer lockedAcquireBuffer() { + assert _channelState.isLockHeldByCurrentThread(); + if (_aggregate == null) - _aggregate = _channel.getByteBufferPool().acquire(getBufferSize(), _channel.isUseOutputDirectByteBuffers()); + { + _pool = _channel.getByteBufferPool(); + _aggregate = _pool.acquire(getBufferSize(), _channel.isUseOutputDirectByteBuffers()); + } return _aggregate; } - private void releaseBuffer() + private void lockedReleaseBuffer(boolean failure) { + assert _channelState.isLockHeldByCurrentThread(); + if (_aggregate != null) { - _aggregate.release(); + if (failure && _pool != null) + _pool.removeAndRelease(_aggregate); + else + _aggregate.release(); _aggregate = null; + _pool = null; } } @@ -821,7 +834,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable // Should we aggregate? if (aggregate) { - acquireBuffer(); + lockedAcquireBuffer(); int filled = BufferUtil.fill(_aggregate.getByteBuffer(), b, off, len); // return if we are not complete, not full and filled all the content @@ -1026,7 +1039,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable } _written = written; - acquireBuffer(); + lockedAcquireBuffer(); BufferUtil.append(_aggregate.getByteBuffer(), (byte)b); } @@ -1431,6 +1444,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable { try (AutoLock l = _channelState.lock()) { + lockedReleaseBuffer(_state != State.CLOSED); _state = State.OPEN; _apiState = ApiState.BLOCKING; _softClose = true; // Stay closed until next request @@ -1440,7 +1454,6 @@ public class HttpOutput extends ServletOutputStream implements Runnable _commitSize = config.getOutputAggregationSize(); if (_commitSize > _bufferSize) _commitSize = _bufferSize; - releaseBuffer(); _written = 0; _writeListener = null; _onError = null;