Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CXF clients based on java.net.http.HttpClient leak threads #1090

Merged
merged 1 commit into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,14 @@ quarkus.cxf.client.proxiedCalculator.proxy-server = ${cxf.it.calculator.proxy.ho
quarkus.cxf.client.proxiedCalculator.proxy-server-port = ${cxf.it.calculator.proxy.port}
quarkus.cxf.client.proxiedCalculator.proxy-username = ${cxf.it.calculator.proxy.user}
quarkus.cxf.client.proxiedCalculator.proxy-password = ${cxf.it.calculator.proxy.password}


quarkus.cxf.client.requestScopedHttpClient.wsdl = ${cxf.it.calculator.baseUri}/calculator-ws/CalculatorService?wsdl
quarkus.cxf.client.requestScopedHttpClient.client-endpoint-url = ${cxf.it.calculator.baseUri}/calculator-ws/CalculatorService
quarkus.cxf.client.requestScopedHttpClient.http-conduit-factory = HttpClientHTTPConduitFactory

quarkus.cxf.client.requestScopedUrlConnectionClient.wsdl = ${cxf.it.calculator.baseUri}/calculator-ws/CalculatorService?wsdl
quarkus.cxf.client.requestScopedUrlConnectionClient.client-endpoint-url = ${cxf.it.calculator.baseUri}/calculator-ws/CalculatorService
quarkus.cxf.client.requestScopedUrlConnectionClient.http-conduit-factory = URLConnectionHTTPConduitFactory
# Get rid of the Could not find endpoint/port warning
quarkus.log.category."org.apache.cxf.frontend.AbstractWSDLBasedEndpointFactory".level = ERROR
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,34 @@ Use the property below to prevent launching the HTTP server at startup:
----
quarkus.http.host-enabled = false
----

[[resource-leaks]]
== Prevent resource leaks

CXF client proxies implement `java.io.Closeable`.
Therefore, it is important to call `((Closeable) proxy).close()` once the client is not needed anymore
to free all associated system resources, such as threads.

{quarkus-cxf-project-name} takes care for closing the clients injected via `@io.quarkiverse.cxf.annotation.CXFClient` automatically
as soon as they are disposed by the CDI container.

For client proxies created manually, it is up to you to call `((Closeable) proxy).close()`:

[source,java]
----
import java.net.URL;
import javax.xml.namespace.QName;
import jakarta.xml.ws.Service;
import java.io.Closeable;

final URL serviceUrl = new URL("http://localhost/myService?wsdl");
final QName qName = new QName("http://acme.org/myNamespace", "MyService");
final Service service = jakarta.xml.ws.Service.create(serviceUrl, qName);
final MyService proxy = service.getPort(MyService.class);

try {
proxy.doSomething();
} finally {
((Closeable) proxy).close();
}
----
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import java.util.stream.Collectors;

import jakarta.enterprise.context.ApplicationScoped;
import jakarta.enterprise.inject.Disposes;
import jakarta.enterprise.inject.Produces;
import jakarta.enterprise.inject.spi.InjectionPoint;
import jakarta.inject.Inject;
Expand Down Expand Up @@ -55,6 +56,7 @@
import io.quarkus.deployment.builditem.nativeimage.NativeImageProxyDefinitionBuildItem;
import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
import io.quarkus.deployment.util.IoUtil;
import io.quarkus.gizmo.AnnotatedElement;
import io.quarkus.gizmo.ClassCreator;
import io.quarkus.gizmo.ClassOutput;
import io.quarkus.gizmo.FieldCreator;
Expand Down Expand Up @@ -401,6 +403,43 @@ private void generateCxfClientProducer(
createService.returnValue(createService.checkCast(cxfClient, sei));

}

/*
* The client proxy implements Closeable so we need to generate a disposer method
* that calls proxy.close(). This is important because e.g. java.net.http.HttpClient based CXF clients
* have some associated threads that is better to stop immediately.
*/
// create method
// >> @Produces
// >> @CXFClient
// >> void closeClient(@Disposes @CXFClient {SEI} client) { .. }
try (MethodCreator createService = classCreator.getMethodCreator(
"closeClient",
void.class,
sei)) {
AnnotatedElement clientParamAnnotations = createService.getParameterAnnotations(0);
clientParamAnnotations.addAnnotation(Disposes.class);
clientParamAnnotations.addAnnotation(CXFClient.class);

final ResultHandle thisHandle = createService.getThis();
final ResultHandle clientHandle = createService.getMethodParam(0);

MethodDescriptor closeClient = MethodDescriptor.ofMethod(
CxfClientProducer.class,
"closeCxfClient",
void.class,
Object.class);
// >> .. {
// >> this.closeCxfClient(client);
// >> }

createService.invokeVirtualMethod(
closeClient,
thisHandle,
clientHandle);
createService.returnVoid();

}
}

// Eventually let's produce
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,20 @@ public Object loadCxfClient(InjectionPoint ip, CXFClientData meta) {
return produceCxfClient(selectorCXFClientInfo(config, fixedConfig, ip, meta));
}

/**
* Called from the <code>{SEI}CxfClientProducer.closeClient(@Disposes @CXFClient {SEI} client)</code> generated in
* {@code io.quarkiverse.cxf.deployment.CxfClientProcessor.generateCxfClientProducer()}.
*
* @param client the CXF client to close
*/
public void closeCxfClient(Object client) {
try {
((Closeable) client).close();
} catch (IOException e) {
throw new RuntimeException("Could not close CXF client " + client.getClass().getName(), e);
}
}

/**
* Must be public, otherwise: java.lang.VerifyError: Bad access to protected data in invokevirtual
*/
Expand Down
15 changes: 15 additions & 0 deletions integration-tests/client/README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,18 @@ And then overwrite the existing file with the new content from the container:
----
curl http://localhost:8080/calculator-ws/CalculatorService?wsdl --output src/main/cxf-codegen-resources/wsdl/CalculatorService.wsdl
----

[[soak]]
== `CxfClientTest.soakRequestScopedHttpClient()` and `CxfClientTest.soakRequestScopedUrlConnectionClient()`

The number of iterations performed by these tests can be set via

[source,shell]
----
$ export QUARKUS_CXF_CLIENT_SOAK_ITERATIONS=100000
$ mvn clean test -Dtest=CxfClientTest#soakRequestScopedHttpClient
$ mvn clean test -Dtest=CxfClientTest#soakRequestScopedUrlConnectionClient
----

The hard-coded default of 300 is intentionally low to make the test pass on Quarkus Platform CI.
Higher values are more likely to uncover resource leaks.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.io.StringWriter;
import java.util.List;

import jakarta.enterprise.context.RequestScoped;
import jakarta.inject.Inject;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.POST;
Expand Down Expand Up @@ -59,6 +60,20 @@ public class CxfClientResource {
@CXFClient("proxiedCalculator")
CalculatorService proxiedCalculator;

@Inject
RequestScopedClients requestScopedClients;

@GET
@Path("/calculator/{client}/add")
@Produces(MediaType.TEXT_PLAIN)
public Response add(@PathParam("client") String client, @QueryParam("a") int a, @QueryParam("b") int b) {
try {
return Response.ok(getClient(client).add(a, b)).build();
} catch (Exception e) {
return Response.serverError().entity(e.getMessage()).build();
}
}

@GET
@Path("/calculator/{client}/multiply")
@Produces(MediaType.TEXT_PLAIN)
Expand Down Expand Up @@ -135,7 +150,7 @@ private CalculatorService getClient(String client) {
case "proxiedCalculator":
return proxiedCalculator;
default:
throw new IllegalStateException("Unexpected client key " + client);
return requestScopedClients.getClient(client);
}
}

Expand Down Expand Up @@ -193,4 +208,27 @@ public String createEscapeHandler(@PathParam("className") String className, Stri
return out.toString();
}

@RequestScoped
public static class RequestScopedClients {

@Inject
@CXFClient("requestScopedHttpClient")
CalculatorService requestScopedHttpClient;

@Inject
@CXFClient("requestScopedUrlConnectionClient")
CalculatorService requestScopedUrlConnectionClient;

public CalculatorService getClient(String client) {
switch (client) {
case "requestScopedHttpClient":
return requestScopedHttpClient;
case "requestScopedUrlConnectionClient":
return requestScopedUrlConnectionClient;
default:
throw new IllegalStateException("Unexpected client key " + client);
}
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,14 @@ quarkus.cxf.client.proxiedCalculator.proxy-server = ${cxf.it.calculator.proxy.ho
quarkus.cxf.client.proxiedCalculator.proxy-server-port = ${cxf.it.calculator.proxy.port}
quarkus.cxf.client.proxiedCalculator.proxy-username = ${cxf.it.calculator.proxy.user}
quarkus.cxf.client.proxiedCalculator.proxy-password = ${cxf.it.calculator.proxy.password}


quarkus.cxf.client.requestScopedHttpClient.wsdl = ${cxf.it.calculator.baseUri}/calculator-ws/CalculatorService?wsdl
quarkus.cxf.client.requestScopedHttpClient.client-endpoint-url = ${cxf.it.calculator.baseUri}/calculator-ws/CalculatorService
quarkus.cxf.client.requestScopedHttpClient.http-conduit-factory = HttpClientHTTPConduitFactory

quarkus.cxf.client.requestScopedUrlConnectionClient.wsdl = ${cxf.it.calculator.baseUri}/calculator-ws/CalculatorService?wsdl
quarkus.cxf.client.requestScopedUrlConnectionClient.client-endpoint-url = ${cxf.it.calculator.baseUri}/calculator-ws/CalculatorService
quarkus.cxf.client.requestScopedUrlConnectionClient.http-conduit-factory = URLConnectionHTTPConduitFactory
# Get rid of the Could not find endpoint/port warning
quarkus.log.category."org.apache.cxf.frontend.AbstractWSDLBasedEndpointFactory".level = ERROR
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,19 @@

import java.io.IOException;
import java.io.InputStream;
import java.net.HttpURLConnection;
import java.net.http.HttpClient;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Optional;
import java.util.Random;

import org.apache.commons.io.IOUtils;
import org.assertj.core.api.Assertions;
import org.eclipse.microprofile.config.ConfigProvider;
import org.jboss.logging.Logger;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
Expand All @@ -26,6 +31,8 @@
@QuarkusTestResource(CxfClientTestResource.class)
public class CxfClientTest {

private static final Logger log = Logger.getLogger(CxfClientTest.class);

@Test
void add() {
RestAssured.given()
Expand Down Expand Up @@ -359,4 +366,47 @@
.body(is("Tom & Jerry"));
}

/**
* Make sure that a request scoped client backed by {@link HttpClient} does not leak threads
* - see <a href=
* "https://github.com/quarkiverse/quarkus-cxf/issues/992">https://github.com/quarkiverse/quarkus-cxf/issues/992</a>.
*/
@Test
void soakRequestScopedHttpClient() {
soak("requestScopedHttpClient");

}

/**
* Make sure that a request scoped client backed by {@link HttpURLConnection} does not leak threads.
*/
@Test
void soakRequestScopedUrlConnectionClient() {
soak("requestScopedUrlConnectionClient");

}

private void soak(String client) {

final Random rnd = new Random();
// we divide by 2 to avoid overflow
int a = rnd.nextInt() / 2;
int b = rnd.nextInt() / 2;
int expected = a + b;

final int requestCount = Integer
.parseInt(Optional.ofNullable(System.getenv("QUARKUS_CXF_CLIENT_SOAK_ITERATIONS")).orElse("300"));
Dismissed Show dismissed Hide dismissed
log.infof("Performing %d interations", requestCount);
for (int i = 0; i < requestCount; i++) {
log.infof("Soaking round %d", i);
RestAssured.given()
.queryParam("a", a)
.queryParam("b", b)
.get("/cxf/client/calculator/" + client + "/add")
.then()
.statusCode(200)
.body(is(String.valueOf(expected)));
}
}

}
2 changes: 2 additions & 0 deletions integration-tests/mtom/README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ $ mvn clean test -Dtest=MtomTest#soak
The hard-coded default of 300 is intentionally low to make the test pass on Quarkus Platform CI.
Higher values are more likely to uncover resource leaks.

Note that there are two more soak tests in CxfClientTest - see link:../client/README.adoc#soak[../client/README.adoc].
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


== `MtomTest.largeAttachment()`

`MtomTest.largeAttachment()` can be tuned via
Expand Down