From c0bb0272f7776c167fa17dcc5fb350e3a57bebdb Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 3 Nov 2022 17:38:22 +1100 Subject: [PATCH] fix release in CachingHttpContentFactory and always use ByteBufferPool Signed-off-by: Lachlan Roberts --- .../jetty/http/CachingHttpContentFactory.java | 65 ++++++++++++------- .../http/FileMappedHttpContentFactory.java | 8 +-- .../org/eclipse/jetty/http/HttpContent.java | 4 +- .../jetty/http/HttpContentWrapper.java | 6 +- .../jetty/http/PreCompressedHttpContent.java | 6 +- .../jetty/http/ResourceHttpContent.java | 4 +- .../http/ValidatingCachingContentFactory.java | 11 +++- .../eclipse/jetty/server/ResourceService.java | 27 ++++---- .../jetty/server/handler/ResourceHandler.java | 21 +++++- .../server/handler/ResourceHandlerTest.java | 2 +- .../jetty/ee10/servlet/DefaultServlet.java | 17 ++++- .../eclipse/jetty/ee9/nested/HttpOutput.java | 6 +- .../jetty/ee9/nested/ResourceHandler.java | 18 ++++- .../jetty/ee9/nested/ResourceService.java | 14 +--- .../resource/ByteBufferRangeWriter.java | 8 +-- .../resource/HttpContentRangeWriter.java | 4 +- .../jetty/ee9/servlet/DefaultServlet.java | 17 ++++- 17 files changed, 156 insertions(+), 82 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingHttpContentFactory.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingHttpContentFactory.java index 9639a057b4c..1041de330b0 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingHttpContentFactory.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingHttpContentFactory.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.http; import java.io.IOException; +import java.nio.ByteBuffer; import java.time.Instant; import java.util.Set; import java.util.SortedSet; @@ -21,11 +22,11 @@ import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.NoopByteBufferPool; -import org.eclipse.jetty.io.RetainableByteBuffer; -import org.eclipse.jetty.io.RetainableByteBufferPool; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.NanoTime; import org.eclipse.jetty.util.StringUtil; @@ -52,20 +53,15 @@ public class CachingHttpContentFactory implements HttpContent.Factory private final HttpContent.Factory _authority; private final ConcurrentMap _cache = new ConcurrentHashMap<>(); private final AtomicLong _cachedSize = new AtomicLong(); - private final RetainableByteBufferPool _byteBufferPool; + private final ByteBufferPool _byteBufferPool; private int _maxCachedFileSize = 128 * 1024 * 1024; private int _maxCachedFiles = 2048; private int _maxCacheSize = 256 * 1024 * 1024; - public CachingHttpContentFactory(HttpContent.Factory authority) - { - this(authority, null); - } - - public CachingHttpContentFactory(HttpContent.Factory authority, RetainableByteBufferPool byteBufferPool) + public CachingHttpContentFactory(HttpContent.Factory authority, ByteBufferPool byteBufferPool) { _authority = authority; - _byteBufferPool = (byteBufferPool == null) ? new NoopByteBufferPool().asRetainableByteBufferPool() : byteBufferPool; + _byteBufferPool = (byteBufferPool == null) ? new NoopByteBufferPool() : byteBufferPool; } protected ConcurrentMap getCache() @@ -208,7 +204,10 @@ public class CachingHttpContentFactory implements HttpContent.Factory { cachingHttpContent.setLastAccessedNanos(NanoTime.now()); if (cachingHttpContent.isValid()) + { + cachingHttpContent.retain(); return (cachingHttpContent instanceof NotFoundHttpContent) ? null : cachingHttpContent; + } else removeFromCache(cachingHttpContent); } @@ -225,6 +224,7 @@ public class CachingHttpContentFactory implements HttpContent.Factory return cachingContent; }); + cachingHttpContent.retain(); if (wasAdded.get()) shrinkCache(); return (cachingHttpContent instanceof NotFoundHttpContent) ? null : cachingHttpContent; @@ -251,11 +251,13 @@ public class CachingHttpContentFactory implements HttpContent.Factory String getKey(); boolean isValid(); + + void retain(); } protected class CachedHttpContent extends HttpContentWrapper implements CachingHttpContent { - private final RetainableByteBuffer _buffer; + private final ByteBuffer _buffer; private final String _cacheKey; private final HttpField _etagField; private final long _contentLengthValue; @@ -267,6 +269,7 @@ public class CachingHttpContentFactory implements HttpContent.Factory private final HttpField _contentLength; private final Instant _lastModifiedInstant; private final HttpField _lastModified; + private final AtomicInteger _referenceCount = new AtomicInteger(1); public CachedHttpContent(String key, HttpContent httpContent) { @@ -283,25 +286,29 @@ public class CachingHttpContentFactory implements HttpContent.Factory _etagField = etagField; // Read the content into memory if the HttpContent does not already have a buffer. - RetainableByteBuffer buffer = httpContent.getBuffer(); - if (buffer != null) + ByteBuffer byteBuffer = httpContent.getByteBuffer(); + if (byteBuffer == null) { try { long contentLengthValue = getContentLengthValue(); if (contentLengthValue > _maxCachedFileSize) { - buffer = _byteBufferPool.acquire((int)contentLengthValue, false); - BufferUtil.readFrom(httpContent.getResource().newReadableByteChannel(), contentLengthValue, buffer.getBuffer()); + byteBuffer = _byteBufferPool.acquire((int)contentLengthValue, false); + BufferUtil.readFrom(httpContent.getResource().newReadableByteChannel(), contentLengthValue, byteBuffer); } } catch (Throwable t) { - buffer = null; + if (byteBuffer != null) + { + _byteBufferPool.release(byteBuffer); + byteBuffer = null; + } LOG.warn("Failed to read Resource", t); } } - _buffer = buffer; + _buffer = byteBuffer; _contentLengthValue = httpContent.getContentLengthValue(); _lastModifiedValue = httpContent.getLastModifiedValue(); @@ -322,7 +329,7 @@ public class CachingHttpContentFactory implements HttpContent.Factory } @Override - public RetainableByteBuffer getBuffer() + public ByteBuffer getByteBuffer() { return _buffer; } @@ -345,12 +352,21 @@ public class CachingHttpContentFactory implements HttpContent.Factory return _cacheKey; } + @Override + public void retain() + { + _referenceCount.incrementAndGet(); + } + @Override public void release() { - super.release(); - if (_buffer != null) - _buffer.release(); + if (_referenceCount.decrementAndGet() == 0) + { + if (_buffer != null) + _byteBufferPool.release(_buffer); + super.release(); + } } @Override @@ -529,7 +545,7 @@ public class CachingHttpContentFactory implements HttpContent.Factory } @Override - public RetainableByteBuffer getBuffer() + public ByteBuffer getByteBuffer() { return null; } @@ -550,5 +566,10 @@ public class CachingHttpContentFactory implements HttpContent.Factory { return true; } + + @Override + public void retain() + { + } } } diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/FileMappedHttpContentFactory.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/FileMappedHttpContentFactory.java index f927df1da66..3b9a6a7ffca 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/FileMappedHttpContentFactory.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/FileMappedHttpContentFactory.java @@ -17,7 +17,6 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.util.Objects; -import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.util.BufferUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -53,12 +52,11 @@ public class FileMappedHttpContentFactory implements HttpContent.Factory } @Override - public RetainableByteBuffer getBuffer() + public ByteBuffer getByteBuffer() { try { - ByteBuffer buffer = BufferUtil.toMappedBuffer(content.getResource().getPath()); - return new RetainableByteBuffer(buffer, b -> {}); + return BufferUtil.toMappedBuffer(content.getResource().getPath()); } catch (Throwable t) { @@ -66,7 +64,7 @@ public class FileMappedHttpContentFactory implements HttpContent.Factory LOG.debug("Error getting Mapped Buffer", t); } - return super.getBuffer(); + return super.getByteBuffer(); } } } diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpContent.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpContent.java index a3a68b90f4d..7556d491644 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpContent.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpContent.java @@ -14,11 +14,11 @@ package org.eclipse.jetty.http; import java.io.IOException; +import java.nio.ByteBuffer; import java.time.Instant; import java.util.Set; import org.eclipse.jetty.http.MimeTypes.Type; -import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.util.resource.Resource; /** @@ -61,7 +61,7 @@ public interface HttpContent Resource getResource(); - RetainableByteBuffer getBuffer(); + ByteBuffer getByteBuffer(); /** * @return Set of available pre-compressed formats for this content, or null if this has not been checked. diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpContentWrapper.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpContentWrapper.java index 8a521e8d001..5d66c4de701 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpContentWrapper.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpContentWrapper.java @@ -13,11 +13,11 @@ package org.eclipse.jetty.http; +import java.nio.ByteBuffer; import java.time.Instant; import java.util.Set; import org.eclipse.jetty.http.MimeTypes.Type; -import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.util.resource.Resource; /** @@ -122,9 +122,9 @@ public class HttpContentWrapper implements HttpContent } @Override - public RetainableByteBuffer getBuffer() + public ByteBuffer getByteBuffer() { - return _delegate.getBuffer(); + return _delegate.getByteBuffer(); } @Override diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/PreCompressedHttpContent.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/PreCompressedHttpContent.java index 481f0609d3f..caa4bfb8513 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/PreCompressedHttpContent.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/PreCompressedHttpContent.java @@ -13,11 +13,11 @@ package org.eclipse.jetty.http; +import java.nio.ByteBuffer; import java.time.Instant; import java.util.Set; import org.eclipse.jetty.http.MimeTypes.Type; -import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.util.resource.Resource; public class PreCompressedHttpContent implements HttpContent @@ -138,9 +138,9 @@ public class PreCompressedHttpContent implements HttpContent } @Override - public RetainableByteBuffer getBuffer() + public ByteBuffer getByteBuffer() { - return _precompressedContent.getBuffer(); + return _precompressedContent.getByteBuffer(); } @Override diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ResourceHttpContent.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ResourceHttpContent.java index 617d387a2e6..75085076b78 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ResourceHttpContent.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ResourceHttpContent.java @@ -13,12 +13,12 @@ package org.eclipse.jetty.http; +import java.nio.ByteBuffer; import java.nio.file.Path; import java.time.Instant; import java.util.Set; import org.eclipse.jetty.http.MimeTypes.Type; -import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.util.resource.Resource; /** @@ -138,7 +138,7 @@ public class ResourceHttpContent implements HttpContent } @Override - public RetainableByteBuffer getBuffer() + public ByteBuffer getByteBuffer() { return null; } diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ValidatingCachingContentFactory.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ValidatingCachingContentFactory.java index 400695369ad..10092cfa4f3 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ValidatingCachingContentFactory.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ValidatingCachingContentFactory.java @@ -19,6 +19,7 @@ import java.util.concurrent.ConcurrentMap; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; +import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.util.NanoTime; import org.eclipse.jetty.util.annotation.Name; import org.eclipse.jetty.util.thread.Scheduler; @@ -54,11 +55,13 @@ public class ValidatingCachingContentFactory extends CachingHttpContentFactory i * * @param authority the wrapped {@link HttpContent.Factory} to use. * @param validationTime time between filesystem checks in ms to see if an {@link HttpContent} is still valid (-1 never validate, 0 always validate). + * @param byteBufferPool the {@link org.eclipse.jetty.io.ByteBufferPool} to use. */ public ValidatingCachingContentFactory(@Name("authority") HttpContent.Factory authority, - @Name("validationTime") long validationTime) + @Name("validationTime") long validationTime, + @Name("byteBufferPool") ByteBufferPool byteBufferPool) { - this(authority, validationTime, null, -1, -1); + this(authority, validationTime, byteBufferPool, null, -1, -1); } /** @@ -67,17 +70,19 @@ public class ValidatingCachingContentFactory extends CachingHttpContentFactory i * * @param authority the wrapped {@link HttpContent.Factory} to use. * @param validationTime time between filesystem checks in ms to see if an {@link HttpContent} is still valid (-1 never validate, 0 always validate). + * @param byteBufferPool the {@link org.eclipse.jetty.io.ByteBufferPool} to use. * @param scheduler scheduler to use for the sweeper, can be null to not use sweeper. * @param sweepDelay time between runs of the sweeper in ms (if < 0 never sweep for invalid entries). * @param maxCacheIdleTime amount of time in ms an entry can be unused before evicted by the sweeper (if < 0 never evict unused entries). */ public ValidatingCachingContentFactory(@Name("authority") HttpContent.Factory authority, @Name("validationTime") long validationTime, + @Name("byteBufferPool") ByteBufferPool byteBufferPool, @Name("scheduler") Scheduler scheduler, @Name("sweepDelay") long sweepDelay, @Name("maxCacheIdleTime") long maxCacheIdleTime) { - super(authority); + super(authority, byteBufferPool); _validationTime = validationTime; _scheduler = scheduler; _sweepDelay = sweepDelay; diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java index 9b4f5225b4b..4c5a779d5b7 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java @@ -43,9 +43,8 @@ import org.eclipse.jetty.http.PreEncodedHttpField; import org.eclipse.jetty.http.QuotedCSV; import org.eclipse.jetty.http.QuotedQualityCSV; import org.eclipse.jetty.http.ResourceHttpContent; +import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.Content; -import org.eclipse.jetty.io.RetainableByteBuffer; -import org.eclipse.jetty.io.RetainableByteBufferPool; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; @@ -575,6 +574,7 @@ public class ResourceService private void sendData(Request request, Response response, Callback callback, HttpContent content, List reqRanges) { long contentLength = content.getContentLengthValue(); + callback = Callback.from(callback, content::release); if (LOG.isDebugEnabled()) LOG.debug(String.format("sendData content=%s", content)); @@ -629,11 +629,10 @@ public class ResourceService { try { - RetainableByteBuffer buffer = content.getBuffer(); + ByteBuffer buffer = content.getByteBuffer(); if (buffer != null) { - buffer.retain(); - response.write(true, buffer.getBuffer(), Callback.from(callback, buffer::release)); + response.write(true, buffer, callback); } else { @@ -647,6 +646,7 @@ public class ResourceService } catch (Throwable x) { + content.release(); callback.failed(x); } } @@ -846,11 +846,12 @@ public class ResourceService private final ReadableByteChannel source; private final Content.Sink sink; private final Callback callback; - private final RetainableByteBuffer byteBuffer; + private final ByteBuffer byteBuffer; + private final ByteBufferPool byteBufferPool; public ContentWriterIteratingCallback(HttpContent content, Response target, Callback callback) throws IOException { - RetainableByteBufferPool byteBufferPool = target.getRequest().getComponents().getByteBufferPool().asRetainableByteBufferPool(); + this.byteBufferPool = target.getRequest().getComponents().getByteBufferPool(); this.source = content.getResource().newReadableByteChannel(); this.sink = target; this.callback = callback; @@ -865,30 +866,30 @@ public class ResourceService if (!source.isOpen()) return Action.SUCCEEDED; - BufferUtil.clearToFill(byteBuffer.getBuffer()); - int read = source.read(byteBuffer.getBuffer()); + BufferUtil.clearToFill(byteBuffer); + int read = source.read(byteBuffer); if (read == -1) { IO.close(source); sink.write(true, BufferUtil.EMPTY_BUFFER, this); return Action.SCHEDULED; } - BufferUtil.flipToFlush(byteBuffer.getBuffer(), 0); - sink.write(false, byteBuffer.getBuffer(), this); + BufferUtil.flipToFlush(byteBuffer, 0); + sink.write(false, byteBuffer, this); return Action.SCHEDULED; } @Override protected void onCompleteSuccess() { - byteBuffer.release(); + byteBufferPool.release(byteBuffer); callback.succeeded(); } @Override protected void onCompleteFailure(Throwable x) { - byteBuffer.release(); + byteBufferPool.release(byteBuffer); callback.failed(x); } } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java index e40233a8c3e..b14bddbcb36 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java @@ -24,10 +24,12 @@ import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.http.PreCompressedHttpContentFactory; import org.eclipse.jetty.http.ResourceHttpContentFactory; import org.eclipse.jetty.http.ValidatingCachingContentFactory; -import org.eclipse.jetty.server.Context; +import org.eclipse.jetty.io.ByteBufferPool; +import org.eclipse.jetty.io.NoopByteBufferPool; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.ResourceService; +import org.eclipse.jetty.server.Server; import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceFactory; @@ -52,6 +54,7 @@ public class ResourceHandler extends Handler.Wrapper { private final ResourceService _resourceService; + private ByteBufferPool _byteBufferPool; private Resource _resourceBase; private MimeTypes _mimeTypes; private List _welcomes = List.of("index.html"); @@ -64,9 +67,9 @@ public class ResourceHandler extends Handler.Wrapper @Override public void doStart() throws Exception { + ContextHandler.Context context = ContextHandler.getCurrentContext(); if (_resourceBase == null) { - Context context = ContextHandler.getCurrentContext(); if (context != null) _resourceBase = context.getBaseResource(); } @@ -75,6 +78,7 @@ public class ResourceHandler extends Handler.Wrapper if (_mimeTypes == null) _mimeTypes = new MimeTypes(); + _byteBufferPool = getByteBufferPool(context); _resourceService.setContentFactory(setupContentFactory()); _resourceService.setWelcomeFactory(setupWelcomeFactory()); if (_resourceService.getStylesheet() == null) @@ -83,12 +87,23 @@ public class ResourceHandler extends Handler.Wrapper super.doStart(); } + private static ByteBufferPool getByteBufferPool(ContextHandler.Context context) + { + if (context == null) + return new NoopByteBufferPool(); + Server server = context.getContextHandler().getServer(); + if (server == null) + return new NoopByteBufferPool(); + ByteBufferPool byteBufferPool = server.getBean(ByteBufferPool.class); + return (byteBufferPool == null) ? new NoopByteBufferPool() : byteBufferPool; + } + protected HttpContent.Factory setupContentFactory() { HttpContent.Factory contentFactory = new ResourceHttpContentFactory(ResourceFactory.of(_resourceBase), _mimeTypes); contentFactory = new PreCompressedHttpContentFactory(contentFactory, _resourceService.getPrecompressedFormats()); contentFactory = new FileMappedHttpContentFactory(contentFactory); - contentFactory = new ValidatingCachingContentFactory(contentFactory, Duration.ofSeconds(1).toMillis()); + contentFactory = new ValidatingCachingContentFactory(contentFactory, Duration.ofSeconds(1).toMillis(), _byteBufferPool); return contentFactory; } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java index b3a4f4999be..113a02bbfd0 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java @@ -669,7 +669,7 @@ public class ResourceHandlerTest HttpContent.Factory contentFactory = new ResourceHttpContentFactory(ResourceFactory.of(getBaseResource()), getMimeTypes()); contentFactory = new PreCompressedHttpContentFactory(contentFactory, getPrecompressedFormats()); contentFactory = new FileMappedHttpContentFactory(contentFactory); - contentFactory = new ValidatingCachingContentFactory(contentFactory, 0); + contentFactory = new ValidatingCachingContentFactory(contentFactory, 0, _local.getByteBufferPool()); return contentFactory; } }; diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java index 62572a5389d..8de95678d87 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java @@ -60,10 +60,13 @@ import org.eclipse.jetty.http.PreCompressedHttpContentFactory; import org.eclipse.jetty.http.ResourceHttpContentFactory; import org.eclipse.jetty.http.ValidatingCachingContentFactory; import org.eclipse.jetty.io.ByteBufferInputStream; +import org.eclipse.jetty.io.ByteBufferPool; +import org.eclipse.jetty.io.NoopByteBufferPool; import org.eclipse.jetty.server.HttpStream; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.ResourceService; import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.util.Blocker; import org.eclipse.jetty.util.BufferUtil; @@ -139,8 +142,9 @@ public class DefaultServlet extends HttpServlet long cacheValidationTime = getInitParameter("cacheValidationTime") != null ? Long.parseLong(getInitParameter("cacheValidationTime")) : -2; if (maxCachedFiles != -2 || maxCacheSize != -2 || maxCachedFileSize != -2 || cacheValidationTime != -2) { + ByteBufferPool byteBufferPool = getByteBufferPool(servletContextHandler); ValidatingCachingContentFactory cached = new ValidatingCachingContentFactory(contentFactory, - (cacheValidationTime > -2) ? cacheValidationTime : Duration.ofSeconds(1).toMillis()); + (cacheValidationTime > -2) ? cacheValidationTime : Duration.ofSeconds(1).toMillis(), byteBufferPool); contentFactory = cached; if (maxCacheSize >= 0) cached.setMaxCacheSize(maxCacheSize); @@ -230,6 +234,17 @@ public class DefaultServlet extends HttpServlet LOG.debug("base resource = {}", _baseResource); } + private static ByteBufferPool getByteBufferPool(ContextHandler contextHandler) + { + if (contextHandler == null) + return new NoopByteBufferPool(); + Server server = contextHandler.getServer(); + if (server == null) + return new NoopByteBufferPool(); + ByteBufferPool byteBufferPool = server.getBean(ByteBufferPool.class); + return (byteBufferPool == null) ? new NoopByteBufferPool() : byteBufferPool; + } + private String getInitParameter(String name, String... deprecated) { String value = super.getInitParameter(name); 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 65e27e4024b..965c8315f51 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 @@ -35,7 +35,6 @@ import jakarta.servlet.WriteListener; import org.eclipse.jetty.http.HttpContent; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.EofException; -import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; @@ -1306,11 +1305,10 @@ public class HttpOutput extends ServletOutputStream implements Runnable if (LOG.isDebugEnabled()) LOG.debug("sendContent(http={},{})", httpContent, callback); - RetainableByteBuffer buffer = httpContent.getBuffer(); + ByteBuffer buffer = httpContent.getByteBuffer(); if (buffer != null) { - buffer.retain(); - sendContent(buffer.getBuffer(), Callback.from(callback, buffer::release)); + sendContent(buffer, callback); return; } diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceHandler.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceHandler.java index 715657b45c1..2d7baf4e582 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceHandler.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceHandler.java @@ -34,6 +34,9 @@ import org.eclipse.jetty.http.PreCompressedHttpContentFactory; import org.eclipse.jetty.http.PreEncodedHttpField; import org.eclipse.jetty.http.ResourceHttpContentFactory; import org.eclipse.jetty.http.ValidatingCachingContentFactory; +import org.eclipse.jetty.io.ByteBufferPool; +import org.eclipse.jetty.io.NoopByteBufferPool; +import org.eclipse.jetty.server.Server; import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceFactory; @@ -51,6 +54,7 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory, { private static final Logger LOG = LoggerFactory.getLogger(ResourceHandler.class); + private ByteBufferPool _byteBufferPool; Resource _baseResource; ContextHandler _context; Resource _defaultStylesheet; @@ -101,18 +105,30 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory, if (_mimeTypes == null) _mimeTypes = _context == null ? new MimeTypes() : _context.getMimeTypes(); + _byteBufferPool = getByteBufferPool(_context); _resourceService.setContentFactory(setupContentFactory()); _resourceService.setWelcomeFactory(this); super.doStart(); } + private static ByteBufferPool getByteBufferPool(ContextHandler contextHandler) + { + if (contextHandler == null) + return new NoopByteBufferPool(); + Server server = contextHandler.getServer(); + if (server == null) + return new NoopByteBufferPool(); + ByteBufferPool byteBufferPool = server.getBean(ByteBufferPool.class); + return (byteBufferPool == null) ? new NoopByteBufferPool() : byteBufferPool; + } + protected HttpContent.Factory setupContentFactory() { HttpContent.Factory contentFactory = new ResourceHttpContentFactory(this, _mimeTypes); contentFactory = new PreCompressedHttpContentFactory(contentFactory, _resourceService.getPrecompressedFormats()); contentFactory = new FileMappedHttpContentFactory(contentFactory); - contentFactory = new ValidatingCachingContentFactory(contentFactory, Duration.ofSeconds(1).toMillis()); + contentFactory = new ValidatingCachingContentFactory(contentFactory, Duration.ofSeconds(1).toMillis(), _byteBufferPool); return contentFactory; } diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceService.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceService.java index 4b069d81be5..d7ed402916e 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceService.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceService.java @@ -17,6 +17,7 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.InvalidPathException; @@ -48,7 +49,6 @@ import org.eclipse.jetty.http.PreCompressedHttpContent; import org.eclipse.jetty.http.PreEncodedHttpField; import org.eclipse.jetty.http.QuotedCSV; import org.eclipse.jetty.http.QuotedQualityCSV; -import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.io.WriterOutputStream; import org.eclipse.jetty.server.ResourceListing; import org.eclipse.jetty.util.BufferUtil; @@ -864,18 +864,10 @@ public class ResourceService if (start == 0 && content.getResource().length() == contentLength) { // attempt efficient ByteBuffer based write for whole content - RetainableByteBuffer buffer = content.getBuffer(); + ByteBuffer buffer = content.getByteBuffer(); if (buffer != null) { - try - { - buffer.retain(); - BufferUtil.writeTo(buffer.getBuffer(), out); - } - finally - { - buffer.release(); - } + BufferUtil.writeTo(buffer, out); return; } diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/resource/ByteBufferRangeWriter.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/resource/ByteBufferRangeWriter.java index cced6107480..c95618d5874 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/resource/ByteBufferRangeWriter.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/resource/ByteBufferRangeWriter.java @@ -17,7 +17,6 @@ import java.io.IOException; import java.io.OutputStream; import java.nio.ByteBuffer; -import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.util.BufferUtil; /** @@ -25,9 +24,9 @@ import org.eclipse.jetty.util.BufferUtil; */ public class ByteBufferRangeWriter implements RangeWriter { - private final RetainableByteBuffer buffer; + private final ByteBuffer buffer; - public ByteBufferRangeWriter(RetainableByteBuffer buffer) + public ByteBufferRangeWriter(ByteBuffer buffer) { this.buffer = buffer; } @@ -35,7 +34,6 @@ public class ByteBufferRangeWriter implements RangeWriter @Override public void close() throws IOException { - buffer.release(); } @Override @@ -51,7 +49,7 @@ public class ByteBufferRangeWriter implements RangeWriter throw new IllegalArgumentException("Unsupported length " + skipTo + " > " + Integer.MAX_VALUE); } - ByteBuffer src = buffer.getBuffer().slice(); + ByteBuffer src = buffer.slice(); src.position((int)skipTo); src.limit(Math.addExact((int)skipTo, (int)length)); BufferUtil.writeTo(src, outputStream); diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/resource/HttpContentRangeWriter.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/resource/HttpContentRangeWriter.java index 1226b5858fe..72918195ff8 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/resource/HttpContentRangeWriter.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/resource/HttpContentRangeWriter.java @@ -14,11 +14,11 @@ package org.eclipse.jetty.ee9.nested.resource; import java.io.IOException; +import java.nio.ByteBuffer; import java.nio.file.Files; import java.util.Objects; import org.eclipse.jetty.http.HttpContent; -import org.eclipse.jetty.io.RetainableByteBuffer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -40,7 +40,7 @@ public class HttpContentRangeWriter Objects.requireNonNull(content, "HttpContent"); // Try direct buffer - RetainableByteBuffer buffer = content.getBuffer(); + ByteBuffer buffer = content.getByteBuffer(); if (buffer != null) return new ByteBufferRangeWriter(buffer); diff --git a/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/DefaultServlet.java b/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/DefaultServlet.java index 319a1a8cbfb..21d5460042f 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/DefaultServlet.java +++ b/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/DefaultServlet.java @@ -40,6 +40,9 @@ import org.eclipse.jetty.http.PreCompressedHttpContentFactory; import org.eclipse.jetty.http.PreEncodedHttpField; import org.eclipse.jetty.http.ResourceHttpContentFactory; import org.eclipse.jetty.http.ValidatingCachingContentFactory; +import org.eclipse.jetty.io.ByteBufferPool; +import org.eclipse.jetty.io.NoopByteBufferPool; +import org.eclipse.jetty.server.Server; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.resource.Resource; @@ -257,8 +260,9 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory, Welc long cacheValidationTime = getInitParameter("cacheValidationTime") != null ? Long.parseLong(getInitParameter("cacheValidationTime")) : -2; if (maxCachedFiles != -2 || maxCacheSize != -2 || maxCachedFileSize != -2 || cacheValidationTime != -2) { + ByteBufferPool byteBufferPool = getByteBufferPool(_contextHandler); _cachingContentFactory = new ValidatingCachingContentFactory(contentFactory, - (cacheValidationTime > -2) ? cacheValidationTime : Duration.ofSeconds(1).toMillis()); + (cacheValidationTime > -2) ? cacheValidationTime : Duration.ofSeconds(1).toMillis(), byteBufferPool); contentFactory = _cachingContentFactory; if (maxCacheSize >= 0) _cachingContentFactory.setMaxCacheSize(maxCacheSize); @@ -296,6 +300,17 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory, Welc LOG.debug("resource base = {}", _baseResource); } + private static ByteBufferPool getByteBufferPool(ContextHandler contextHandler) + { + if (contextHandler == null) + return new NoopByteBufferPool(); + Server server = contextHandler.getServer(); + if (server == null) + return new NoopByteBufferPool(); + ByteBufferPool byteBufferPool = server.getBean(ByteBufferPool.class); + return (byteBufferPool == null) ? new NoopByteBufferPool() : byteBufferPool; + } + private String getInitParameter(String name, String... deprecated) { String value = super.getInitParameter(name);