diff --git a/core/src/main/java/feign/AsyncFeign.java b/core/src/main/java/feign/AsyncFeign.java index 08cfcc96c..38830b423 100644 --- a/core/src/main/java/feign/AsyncFeign.java +++ b/core/src/main/java/feign/AsyncFeign.java @@ -56,23 +56,28 @@ public static AsyncBuilder asyncBuilder() { return builder(); } - private static class LazyInitializedExecutorService { - - private static final ExecutorService instance = - Executors.newCachedThreadPool( - r -> { - final Thread result = new Thread(r); - result.setDaemon(true); - return result; - }); + /** + * Creates the default {@link ExecutorService} used when neither a custom {@link AsyncClient} nor + * a custom executor is provided. The executor is created per built client (instance scoped) + * rather than being held in a static singleton, so that it - together with its daemon threads - + * becomes eligible for garbage collection once the owning client is discarded. This avoids the + * {@code ClassLoader} leak that a static singleton causes on application redeployments inside + * servlet containers (see gh-3178). + */ + private static ExecutorService defaultExecutorService() { + return Executors.newCachedThreadPool( + r -> { + final Thread result = new Thread(r); + result.setDaemon(true); + return result; + }); } public static class AsyncBuilder extends BaseBuilder, AsyncFeign> { private AsyncContextSupplier defaultContextSupplier = () -> null; - private AsyncClient client = - new DefaultAsyncClient<>( - new DefaultClient(null, null), LazyInitializedExecutorService.instance); + private AsyncClient client; + private ExecutorService executorService; private MethodInfoResolver methodInfoResolver = MethodInfo::new; @Deprecated @@ -86,6 +91,28 @@ public AsyncBuilder client(AsyncClient client) { return this; } + /** + * Sets the {@link ExecutorService} used by the default {@link AsyncClient} to run the + * (blocking) underlying client calls. When provided, the caller owns the executor's lifecycle + * and is responsible for shutting it down. This is the recommended way to avoid the {@code + * ClassLoader} leak described in gh-3178: supply a managed, shut-downable executor instead of + * relying on the built-in default. Ignored when a custom {@link #client(AsyncClient)} is + * supplied. + */ + public AsyncBuilder executorService(ExecutorService executorService) { + this.executorService = executorService; + return this; + } + + private AsyncClient resolveClient() { + if (client != null) { + return client; + } + final ExecutorService executor = + executorService != null ? executorService : defaultExecutorService(); + return new DefaultAsyncClient<>(new DefaultClient(null, null), executor); + } + public AsyncBuilder methodInfoResolver(MethodInfoResolver methodInfoResolver) { this.methodInfoResolver = methodInfoResolver; return this; @@ -206,6 +233,7 @@ public AsyncBuilder invocationHandlerFactory( @Override public AsyncFeign internalBuild() { + final AsyncClient client = resolveClient(); AsyncResponseHandler responseHandler = (AsyncResponseHandler) Capability.enrich( diff --git a/core/src/main/java/feign/BaseBuilder.java b/core/src/main/java/feign/BaseBuilder.java index 05608dc2e..d279ed764 100644 --- a/core/src/main/java/feign/BaseBuilder.java +++ b/core/src/main/java/feign/BaseBuilder.java @@ -362,6 +362,8 @@ List getFieldsToEnrich() { .filter(field -> !Objects.equals(field.getName(), "requestInterceptors")) .filter(field -> !Objects.equals(field.getName(), "responseInterceptors")) .filter(field -> !Objects.equals(field.getName(), "methodInterceptors")) + // caller-owned lifecycle resources are not capability-enriched + .filter(field -> !Objects.equals(field.getName(), "executorService")) // skip primitive types .filter(field -> !field.getType().isPrimitive()) // skip enumerations diff --git a/core/src/test/java/feign/AsyncFeignTest.java b/core/src/test/java/feign/AsyncFeignTest.java index efd965632..300385be6 100644 --- a/core/src/test/java/feign/AsyncFeignTest.java +++ b/core/src/test/java/feign/AsyncFeignTest.java @@ -24,6 +24,10 @@ import static org.assertj.core.data.MapEntry.entry; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import com.google.gson.Gson; import com.google.gson.reflect.TypeToken; @@ -104,6 +108,39 @@ void postTemplateParamsResolve() throws Exception { + " \"password\"}"); } + @Test + void usesProvidedExecutorService() throws Exception { + server.enqueue(new MockResponse().setBody("foo")); + + ExecutorService executorService = spy(Executors.newCachedThreadPool()); + try { + TestInterfaceAsync api = + AsyncFeign.builder() + .decoder(new DefaultDecoder()) + .executorService(executorService) + .target(TestInterfaceAsync.class, "http://localhost:" + server.getPort()); + + String result = api.post().join(); + + assertThat(result).isEqualTo("foo"); + verify(executorService, atLeastOnce()).submit(any(Runnable.class)); + } finally { + executorService.shutdownNow(); + } + } + + @Test + void defaultExecutorServiceStillExecutesRequests() throws Exception { + server.enqueue(new MockResponse().setBody("foo")); + + TestInterfaceAsync api = + AsyncFeign.builder() + .decoder(new DefaultDecoder()) + .target(TestInterfaceAsync.class, "http://localhost:" + server.getPort()); + + assertThat(api.post().join()).isEqualTo("foo"); + } + @Test void postFormParams() throws Exception { server.enqueue(new MockResponse().setBody("foo"));