Skip to content

[java][native] make generated APIs AutoCloseable and close the owned HttpClient - #24960

Open
jtnord wants to merge 1 commit into
OpenAPITools:masterfrom
jtnord:closeable-java-httpClient
Open

jtnord wants to merge 1 commit into
OpenAPITools:masterfrom
jtnord:closeable-java-httpClient

Conversation

@jtnord

@jtnord jtnord commented Sep 17, 2026

Copy link
Copy Markdown

ApiClient.getHttpClient() returns builder.build(), so every native API instance is handed a freshly built HttpClient that no other instance shares. That client was never released, leaking its connection pool and threads for the lifetime of the process.

Generated API classes now implement AutoCloseable and close that client in close(), so they can be used in try-with-resources. HttpClient only became AutoCloseable in Java 21 while the native library targets Java 11, so the call is guarded by an instanceof check; on older runtimes there is no lifecycle API to call and close() is a no-op.

The other Java client libraries are intentionally left alone. They hold a shared ApiClient (usually the process-wide Configuration singleton) or wrap a client that is not closeable at all, so an API instance owns nothing it would be safe to close.

Note the current master does not build cleanly for me on windows either natively or in WSL, so several tests will only have been run in CI (tried with a variety of different JVMs, 25 fails with:

KotlinSpringServerCodegenTest.testXMinimumMessageAndXMaximumMessage_long -- Time elapsed: 0.437 s <<< ERROR!
java.lang.IllegalArgumentException: 25.0.4

11 fails with:

[main] WARN  io.swagger.v3.parser.OpenAPIV3Parser - Exception while reading:
io.swagger.v3.parser.exception.ReadContentException: Unable to read location `src/test/resources/3_0/petstore.yaml` 

followed by

ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.2.5:test (default-test) on project openapi-generator: Execution default-test of goal org.apache.maven.plugins:maven-surefire-plugin:3.2.5:test failed: org.junit.platform.commons.JUnitException: TestEngine with ID 'testng' failed to discover tests:
[ERROR] An error occurred while instantiating class org.openapitools.codegen.java.AbstractJavaCodegenTest: null: ExceptionInInitializerError: NullPointerException

fixes: #24942

@bbdouglas @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger @karismann @Zomzog @lwlee2608 @martin-mfg @KannaKim

PR checklist

  • Read the contribution guidelines.
  • Run the following to build the project and update samples:
    ./mvnw clean package || exit
    ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
    ./bin/utils/export_docs_generators.sh || exit
    
    (For Windows users, please run the script in WSL)
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

…HttpClient

ApiClient.getHttpClient() returns builder.build(), so every native API
instance is handed a freshly built HttpClient that no other instance
shares. That client was never released, leaking its connection pool and
threads for the lifetime of the process.

Generated API classes now implement AutoCloseable and close that client
in close(), so they can be used in try-with-resources. HttpClient only
became AutoCloseable in Java 21 while the native library targets Java
11, so the call is guarded by an instanceof check; on older runtimes
there is no lifecycle API to call and close() is a no-op.

The other Java client libraries are intentionally left alone. They hold
a shared ApiClient (usually the process-wide Configuration singleton) or
wrap a client that is not closeable at all, so an API instance owns
nothing it would be safe to close.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

4 issues found across 36 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="modules/openapi-generator/src/main/resources/Java/libraries/native/api.mustache">

<violation number="1" location="modules/openapi-generator/src/main/resources/Java/libraries/native/api.mustache:968">
P1: When a specification has a parameterless operationId `close`, this template emits both the endpoint method and this lifecycle method with the same Java signature, so generation produces uncompilable code. Reserve or rename colliding operation IDs before rendering the API methods.</violation>

<violation number="2" location="modules/openapi-generator/src/main/resources/Java/libraries/native/api.mustache:968">
P2: Generated APIs force try-with-resources callers to handle checked `Exception`, although Java 21 `HttpClient.close()` has no checked exception. Remove `throws Exception` and translate the reflective/interface close failure to an unchecked exception.</violation>
</file>

<file name="samples/client/petstore/java/native/src/main/java/org/openapitools/client/api/UserApi.java">

<violation number="1" location="samples/client/petstore/java/native/src/main/java/org/openapitools/client/api/UserApi.java:1087">
P2: On Java 21+ this close() permanently shuts down the HttpClient, but the "owned exclusively" assumption is only true for the base ApiClient implementation. `ApiClient.getHttpClient()` is public and overridable (`protected HttpClient.Builder builder` is also meant for extension): a subclass that caches one shared HttpClient to avoid the per-instance leak this PR describes makes every API instance constructed from it share that client, and closing any one instance will close the client all other instances (and the subclass's own uses) depend on, failing their next request with IllegalStateException. The Javadoc asserts exclusivity without any way for close() to verify it. Document this contract on `ApiClient.getHttpClient()`/the class so subclasses and shared-client configurations are not silently broken, or capture ownership explicitly at construction.</violation>
</file>

<file name="samples/client/echo_api/java/native/src/main/java/org/openapitools/client/api/AuthApi.java">

<violation number="1" location="samples/client/echo_api/java/native/src/main/java/org/openapitools/client/api/AuthApi.java:380">
P3: The new close()-based resource lifecycle has no test coverage in this sample: AuthApiTest.java never calls close() or exercises the AutoCloseable path. Since this sample compiles with maven.compiler.source/target = 11, the `instanceof AutoCloseable` guard is always false on the baseline JVM, so the added behavior is silently a no-op in every test/CI run and would only be exercised on a Java 21+ runtime. Add a test (e.g., in AuthApiTest) that constructs an API instance, calls close(), and verifies it doesn't throw, at minimum catching regressions in the close() code path on any runtime.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

* on earlier runtimes there is no way to release the client's threads early.
*/
@Override
public void close() throws Exception {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1: When a specification has a parameterless operationId close, this template emits both the endpoint method and this lifecycle method with the same Java signature, so generation produces uncompilable code. Reserve or rename colliding operation IDs before rendering the API methods.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/Java/libraries/native/api.mustache, line 968:

<comment>When a specification has a parameterless operationId `close`, this template emits both the endpoint method and this lifecycle method with the same Java signature, so generation produces uncompilable code. Reserve or rename colliding operation IDs before rendering the API methods.</comment>

<file context>
@@ -955,5 +955,20 @@ public class {{classname}} {
+   * on earlier runtimes there is no way to release the client's threads early.
+   */
+  @Override
+  public void close() throws Exception {
+    if (memberVarHttpClient instanceof AutoCloseable) {
+      ((AutoCloseable) memberVarHttpClient).close();
</file context>

*/
@Override
public void close() throws Exception {
if (memberVarHttpClient instanceof AutoCloseable) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: On Java 21+ this close() permanently shuts down the HttpClient, but the "owned exclusively" assumption is only true for the base ApiClient implementation. ApiClient.getHttpClient() is public and overridable (protected HttpClient.Builder builder is also meant for extension): a subclass that caches one shared HttpClient to avoid the per-instance leak this PR describes makes every API instance constructed from it share that client, and closing any one instance will close the client all other instances (and the subclass's own uses) depend on, failing their next request with IllegalStateException. The Javadoc asserts exclusivity without any way for close() to verify it. Document this contract on ApiClient.getHttpClient()/the class so subclasses and shared-client configurations are not silently broken, or capture ownership explicitly at construction.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/java/native/src/main/java/org/openapitools/client/api/UserApi.java, line 1087:

<comment>On Java 21+ this close() permanently shuts down the HttpClient, but the "owned exclusively" assumption is only true for the base ApiClient implementation. `ApiClient.getHttpClient()` is public and overridable (`protected HttpClient.Builder builder` is also meant for extension): a subclass that caches one shared HttpClient to avoid the per-instance leak this PR describes makes every API instance constructed from it share that client, and closing any one instance will close the client all other instances (and the subclass's own uses) depend on, failing their next request with IllegalStateException. The Javadoc asserts exclusivity without any way for close() to verify it. Document this contract on `ApiClient.getHttpClient()`/the class so subclasses and shared-client configurations are not silently broken, or capture ownership explicitly at construction.</comment>

<file context>
@@ -1073,4 +1073,19 @@ private HttpRequest.Builder updateUserRequestBuilder(@javax.annotation.Nonnull S
+   */
+  @Override
+  public void close() throws Exception {
+    if (memberVarHttpClient instanceof AutoCloseable) {
+      ((AutoCloseable) memberVarHttpClient).close();
+    }
</file context>

Comment on lines +968 to +972
public void close() throws Exception {
if (memberVarHttpClient instanceof AutoCloseable) {
((AutoCloseable) memberVarHttpClient).close();
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Generated APIs force try-with-resources callers to handle checked Exception, although Java 21 HttpClient.close() has no checked exception. Remove throws Exception and translate the reflective/interface close failure to an unchecked exception.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/Java/libraries/native/api.mustache, line 968:

<comment>Generated APIs force try-with-resources callers to handle checked `Exception`, although Java 21 `HttpClient.close()` has no checked exception. Remove `throws Exception` and translate the reflective/interface close failure to an unchecked exception.</comment>

<file context>
@@ -955,5 +955,20 @@ public class {{classname}} {
+   * on earlier runtimes there is no way to release the client's threads early.
+   */
+  @Override
+  public void close() throws Exception {
+    if (memberVarHttpClient instanceof AutoCloseable) {
+      ((AutoCloseable) memberVarHttpClient).close();
</file context>
Suggested change
public void close() throws Exception {
if (memberVarHttpClient instanceof AutoCloseable) {
((AutoCloseable) memberVarHttpClient).close();
}
}
public void close() {
if (memberVarHttpClient instanceof AutoCloseable) {
try {
((AutoCloseable) memberVarHttpClient).close();
} catch (RuntimeException e) {
throw e;
} catch (Exception e) {
throw new RuntimeException(e);
}
}
}

* on earlier runtimes there is no way to release the client's threads early.
*/
@Override
public void close() throws Exception {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P3: The new close()-based resource lifecycle has no test coverage in this sample: AuthApiTest.java never calls close() or exercises the AutoCloseable path. Since this sample compiles with maven.compiler.source/target = 11, the instanceof AutoCloseable guard is always false on the baseline JVM, so the added behavior is silently a no-op in every test/CI run and would only be exercised on a Java 21+ runtime. Add a test (e.g., in AuthApiTest) that constructs an API instance, calls close(), and verifies it doesn't throw, at minimum catching regressions in the close() code path on any runtime.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/echo_api/java/native/src/main/java/org/openapitools/client/api/AuthApi.java, line 380:

<comment>The new close()-based resource lifecycle has no test coverage in this sample: AuthApiTest.java never calls close() or exercises the AutoCloseable path. Since this sample compiles with maven.compiler.source/target = 11, the `instanceof AutoCloseable` guard is always false on the baseline JVM, so the added behavior is silently a no-op in every test/CI run and would only be exercised on a Java 21+ runtime. Add a test (e.g., in AuthApiTest) that constructs an API instance, calls close(), and verifies it doesn't throw, at minimum catching regressions in the close() code path on any runtime.</comment>

<file context>
@@ -367,4 +367,19 @@ private HttpRequest.Builder testAuthHttpBearerRequestBuilder(Map<String, String>
+   * on earlier runtimes there is no way to release the client's threads early.
+   */
+  @Override
+  public void close() throws Exception {
+    if (memberVarHttpClient instanceof AutoCloseable) {
+      ((AutoCloseable) memberVarHttpClient).close();
</file context>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] [JAVA] HttpClient memberVarHttpClient not closed

1 participant