diff --git a/providers/multipart/src/main/java/org/jboss/resteasy/plugins/providers/multipart/MultipartFormDataInputImpl.java b/providers/multipart/src/main/java/org/jboss/resteasy/plugins/providers/multipart/MultipartFormDataInputImpl.java index b23b6f3ecbe..a30f7ede4e1 100644 --- a/providers/multipart/src/main/java/org/jboss/resteasy/plugins/providers/multipart/MultipartFormDataInputImpl.java +++ b/providers/multipart/src/main/java/org/jboss/resteasy/plugins/providers/multipart/MultipartFormDataInputImpl.java @@ -86,9 +86,4 @@ protected InputPart extractPart(BodyPart bodyPart) throws IOException { return currPart; } - - protected void finalize() throws Throwable - { - close(); - } } diff --git a/providers/multipart/src/main/java/org/jboss/resteasy/plugins/providers/multipart/MultipartInputImpl.java b/providers/multipart/src/main/java/org/jboss/resteasy/plugins/providers/multipart/MultipartInputImpl.java index 98f0d950181..42d4d55754b 100644 --- a/providers/multipart/src/main/java/org/jboss/resteasy/plugins/providers/multipart/MultipartInputImpl.java +++ b/providers/multipart/src/main/java/org/jboss/resteasy/plugins/providers/multipart/MultipartInputImpl.java @@ -11,6 +11,7 @@ import org.apache.james.mime4j.stream.Field; import org.jboss.logging.Logger; import org.jboss.resteasy.core.ProvidersContextRetainer; +import org.jboss.resteasy.core.ResourceCleaner; import org.jboss.resteasy.core.ResteasyContext; import org.jboss.resteasy.spi.HttpRequest; import org.jboss.resteasy.util.CaseInsensitiveMap; @@ -29,6 +30,7 @@ import java.io.SequenceInputStream; import java.io.UnsupportedEncodingException; import java.lang.annotation.Annotation; +import java.lang.ref.Cleaner; import java.lang.reflect.Type; import java.nio.charset.StandardCharsets; import java.util.ArrayList; @@ -43,9 +45,21 @@ */ public class MultipartInputImpl implements MultipartInput, ProvidersContextRetainer { + private static class CleanupAction implements Runnable { + private volatile Message mimeMessage; + + @Override + public void run() { + final Message message = mimeMessage; + if (message != null) { + message.dispose(); + } + } + } protected MediaType contentType; protected Providers workers; - protected Message mimeMessage; + private final CleanupAction cleanupAction; + private final Cleaner.Cleanable cleanable; protected List parts = new ArrayList(); protected static final Annotation[] empty = {}; protected MediaType defaultPartContentType = MultipartConstants.TEXT_PLAIN_WITH_CHARSET_US_ASCII_TYPE; @@ -56,6 +70,8 @@ public MultipartInputImpl(final MediaType contentType, final Providers workers) { this.contentType = contentType; this.workers = workers; + this.cleanupAction = new CleanupAction(); + cleanable = ResourceCleaner.register(this, cleanupAction); HttpRequest httpRequest = ResteasyContext.getContextData(HttpRequest.class); if (httpRequest != null) { @@ -80,6 +96,8 @@ public MultipartInputImpl(final MediaType contentType, final Providers workers, { this.contentType = contentType; this.workers = workers; + this.cleanupAction = new CleanupAction(); + cleanable = ResourceCleaner.register(this, cleanupAction); if (defaultPartContentType != null) this.defaultPartContentType = defaultPartContentType; this.defaultPartCharset = defaultPartCharset; if (defaultPartCharset != null) @@ -97,11 +115,13 @@ public MultipartInputImpl(final Multipart multipart, final Providers workers) th } } this.workers = workers; + this.cleanupAction = new CleanupAction(); + cleanable = ResourceCleaner.register(this, cleanupAction); } public void parse(InputStream is) throws IOException { - mimeMessage = Mime4JWorkaround.parseMessage(addHeaderToHeadlessStream(is)); + cleanupAction.mimeMessage = Mime4JWorkaround.parseMessage(addHeaderToHeadlessStream(is)); extractParts(); } @@ -121,7 +141,7 @@ protected InputStream createHeaderInputStream() public String getPreamble() { - return ((Multipart) mimeMessage.getBody()).getPreamble(); + return ((Multipart) getMimeMessage().getBody()).getPreamble(); } public List getParts() @@ -131,7 +151,7 @@ public List getParts() protected void extractParts() throws IOException { - Multipart multipart = (Multipart) mimeMessage.getBody(); + Multipart multipart = (Multipart) getMimeMessage().getBody(); for (Entity entity : multipart.getBodyParts()) { if (entity instanceof BodyPart) { @@ -145,6 +165,10 @@ protected InputPart extractPart(BodyPart bodyPart) throws IOException return new PartImpl(bodyPart); } + protected Message getMimeMessage() { + return cleanupAction.mimeMessage; + } + public class PartImpl implements InputPart { @@ -339,22 +363,7 @@ public static void main(String[] args) throws Exception @Override public void close() { - if (mimeMessage != null) - { - try - { - mimeMessage.dispose(); - } - catch (Exception e) - { - - } - } - } - - protected void finalize() throws Throwable - { - close(); + cleanable.clean(); } protected String getCharset(MediaType mediaType) diff --git a/providers/multipart/src/main/java/org/jboss/resteasy/plugins/providers/multipart/MultipartRelatedInputImpl.java b/providers/multipart/src/main/java/org/jboss/resteasy/plugins/providers/multipart/MultipartRelatedInputImpl.java index bc246cb28fc..1f124211b17 100644 --- a/providers/multipart/src/main/java/org/jboss/resteasy/plugins/providers/multipart/MultipartRelatedInputImpl.java +++ b/providers/multipart/src/main/java/org/jboss/resteasy/plugins/providers/multipart/MultipartRelatedInputImpl.java @@ -39,7 +39,7 @@ public MultipartRelatedInputImpl (final Multipart multipart, final Providers wor @Override public void parse(InputStream is) throws IOException { super.parse(is); - ContentTypeField contentTypeField = (ContentTypeField) mimeMessage + ContentTypeField contentTypeField = (ContentTypeField) getMimeMessage() .getHeader().getField(FieldName.CONTENT_TYPE); start = contentTypeField.getParameter("start"); startInfo = contentTypeField.getParameter("start-info"); diff --git a/resteasy-client/src/main/java/org/jboss/resteasy/client/jaxrs/engines/ApacheHttpClient43Engine.java b/resteasy-client/src/main/java/org/jboss/resteasy/client/jaxrs/engines/ApacheHttpClient43Engine.java index 5dd88472785..5bf082ed975 100644 --- a/resteasy-client/src/main/java/org/jboss/resteasy/client/jaxrs/engines/ApacheHttpClient43Engine.java +++ b/resteasy-client/src/main/java/org/jboss/resteasy/client/jaxrs/engines/ApacheHttpClient43Engine.java @@ -2,7 +2,6 @@ import org.apache.http.HttpHost; import org.apache.http.client.HttpClient; -import org.jboss.resteasy.client.jaxrs.i18n.LogMessages; /** * An Apache HTTP engine for use with the new Builder Config style. @@ -34,12 +33,4 @@ public ApacheHttpClient43Engine(final HttpClient httpClient, final HttpContextPr super(httpClient, httpContextProvider); } - public void finalize() throws Throwable - { - if (!closed && allowClosingHttpClient && httpClient != null) - LogMessages.LOGGER.closingForYou(this.getClass()); - close(); - super.finalize(); - } - } diff --git a/resteasy-client/src/main/java/org/jboss/resteasy/client/jaxrs/engines/ManualClosingApacheHttpClient43Engine.java b/resteasy-client/src/main/java/org/jboss/resteasy/client/jaxrs/engines/ManualClosingApacheHttpClient43Engine.java index 86d538299d4..1949f5ae2d3 100644 --- a/resteasy-client/src/main/java/org/jboss/resteasy/client/jaxrs/engines/ManualClosingApacheHttpClient43Engine.java +++ b/resteasy-client/src/main/java/org/jboss/resteasy/client/jaxrs/engines/ManualClosingApacheHttpClient43Engine.java @@ -23,6 +23,7 @@ import org.jboss.resteasy.client.jaxrs.internal.ClientInvocation; import org.jboss.resteasy.client.jaxrs.internal.ClientResponse; import org.jboss.resteasy.client.jaxrs.internal.FinalizedClientResponse; +import org.jboss.resteasy.core.ResourceCleaner; import org.jboss.resteasy.spi.config.ConfigurationFactory; import org.jboss.resteasy.util.CaseInsensitiveMap; @@ -39,6 +40,7 @@ import java.io.IOException; import java.io.InputStream; import java.lang.management.ManagementFactory; +import java.lang.ref.Cleaner; import java.nio.file.Path; import java.security.AccessController; import java.security.PrivilegedAction; @@ -47,12 +49,36 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; /** * An Apache HTTP engine for use with the new Builder Config style. */ public class ManualClosingApacheHttpClient43Engine implements ApacheHttpClientEngine { + private static class CleanupAction implements Runnable { + private final AtomicBoolean closed; + private final CloseableHttpClient client; + + private CleanupAction(final AtomicBoolean closed, final CloseableHttpClient client) { + this.closed = closed; + this.client = client; + } + + @Override + public void run() { + if (closed.compareAndSet(false, true)) { + if (client != null) { + LogMessages.LOGGER.closingForYou(this.getClass()); + try { + client.close(); + } catch (Exception e) { + LogMessages.LOGGER.debugf(e, "Failed to close client %s", client); + } + } + } + } + } static final String FILE_UPLOAD_IN_MEMORY_THRESHOLD_PROPERTY = "org.jboss.resteasy.client.jaxrs.engines.fileUploadInMemoryThreshold"; @@ -79,7 +105,8 @@ public String run() throws Exception protected final HttpClient httpClient; - protected boolean closed; + protected final AtomicBoolean closed; + private final Cleaner.Cleanable cleanable; protected final boolean allowClosingHttpClient; @@ -161,6 +188,8 @@ private ManualClosingApacheHttpClient43Engine(final HttpClient httpClient, } this.httpContextProvider = httpContextProvider; this.allowClosingHttpClient = closeHttpClient; + closed = new AtomicBoolean(false); + this.cleanable = createCleanable(this, closeHttpClient, closed, this.httpClient); this.defaultProxy = defaultProxy; try @@ -681,26 +710,20 @@ private RequestConfig getCurrentConfiguration(final ClientInvocation request, fi public boolean isClosed() { - return closed; + return closed.get(); } - public void close() - { - if (closed) - return; + @Override + public void close() { + cleanable.clean(); + } - if (allowClosingHttpClient && httpClient != null) - { - try - { - ((CloseableHttpClient) httpClient).close(); - } - catch (Exception e) - { - throw new RuntimeException(e); - } + private static Cleaner.Cleanable createCleanable(final ManualClosingApacheHttpClient43Engine engine, final boolean allowClose, + final AtomicBoolean closed, final HttpClient client) { + if (allowClose && client instanceof CloseableHttpClient) { + return ResourceCleaner.register(engine, new CleanupAction(closed, (CloseableHttpClient) client)); } - closed = true; + return () -> closed.set(true); } private static Path getTempDir() { diff --git a/resteasy-client/src/main/java/org/jboss/resteasy/client/jaxrs/internal/FinalizedClientResponse.java b/resteasy-client/src/main/java/org/jboss/resteasy/client/jaxrs/internal/FinalizedClientResponse.java index 4cfeeae18d2..da666fff250 100644 --- a/resteasy-client/src/main/java/org/jboss/resteasy/client/jaxrs/internal/FinalizedClientResponse.java +++ b/resteasy-client/src/main/java/org/jboss/resteasy/client/jaxrs/internal/FinalizedClientResponse.java @@ -5,7 +5,10 @@ /** * A class that adds a {@link Object#finalize) method to the {@link ClientResponse} as a last ditch backstop to prevent * leaking resources with ill-behaved clients. Use of finalize could incur a significant performance penalty. + * + * @deprecated {@linkplain #finalize() finalizers} should no longer be used and this type will be removed in the future */ +@Deprecated public abstract class FinalizedClientResponse extends ClientResponse { protected FinalizedClientResponse(final ClientConfiguration configuration, diff --git a/resteasy-core/src/main/java/org/jboss/resteasy/core/ResourceCleaner.java b/resteasy-core/src/main/java/org/jboss/resteasy/core/ResourceCleaner.java new file mode 100644 index 00000000000..24ea303860b --- /dev/null +++ b/resteasy-core/src/main/java/org/jboss/resteasy/core/ResourceCleaner.java @@ -0,0 +1,50 @@ +/* + * JBoss, Home of Professional Open Source. + * + * Copyright 2022 Red Hat, Inc., and individual contributors + * as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jboss.resteasy.core; + +import java.lang.ref.Cleaner; + +/** + * A utility used to register resources to be cleaned with a {@link Cleaner}. + * + * @author James R. Perkins + */ +public class ResourceCleaner { + + private static final Cleaner CLEANER = Cleaner.create(); + + private ResourceCleaner() { + } + + /** + * Register an object and an action to clean up resources associated with the object once the object instance + * becomes phantom unreachable. + * + * @param instance the instance to monitor + * @param action the action to run for the clean up + * + * @return a cleanable instance + * + * @see Cleaner + */ + public static Cleaner.Cleanable register(final Object instance, final Runnable action) { + return CLEANER.register(instance, action); + } +} diff --git a/testsuite/integration-tests/src/test/java/org/jboss/resteasy/test/client/ClientExecutorShutdownTest.java b/testsuite/integration-tests/src/test/java/org/jboss/resteasy/test/client/ClientExecutorShutdownTest.java index 86f3768f42b..e9f63025533 100644 --- a/testsuite/integration-tests/src/test/java/org/jboss/resteasy/test/client/ClientExecutorShutdownTest.java +++ b/testsuite/integration-tests/src/test/java/org/jboss/resteasy/test/client/ClientExecutorShutdownTest.java @@ -65,7 +65,7 @@ public void testApacheHttpClient4ExecutorNonSharedHttpClientFinalize() throws Th ResteasyClient client = ((ResteasyClientBuilder)ClientBuilder.newBuilder()).httpEngine(engine).build(); Response response = client.target(generateURL("/test")).request().post(null); Assert.assertEquals(HttpResponseCodes.SC_NO_CONTENT, response.getStatus()); - engine.finalize(); + engine.close(); HttpClient httpClient = engine.getHttpClient(); HttpPost post = new HttpPost(generateURL("/test")); try { @@ -114,7 +114,7 @@ public void testApacheHttpClient4ExecutorSharedHttpClientFinalize() throws Throw ResteasyClient client = ((ResteasyClientBuilder)ClientBuilder.newBuilder()).httpEngine(engine).build(); Response response = client.target(generateURL("/test")).request().post(null); Assert.assertEquals(HttpResponseCodes.SC_NO_CONTENT, response.getStatus()); - engine.finalize(); + engine.close(); Assert.assertEquals("Original httpclient and engine httpclient are not the same instance", httpClient, engine.getHttpClient()); HttpPost post = new HttpPost(generateURL("/test"));