From 8177d0bf53aadf8b5aa8a6a3e1caa415dbbb86cb Mon Sep 17 00:00:00 2001 From: Benjamin Buehlmann Date: Tue, 30 Jun 2026 16:56:11 +0200 Subject: [PATCH] Release pooled connection after DELETE to prevent pool exhaustion AbstractApi.delete(...) returned the JAX-RS Response without reading its entity or closing it. Most delete callers are void (e.g. GroupApi.deleteGroup, ProjectApi.deleteProject) and discard the returned Response, so with a pooling/Apache connector (used for proxied clients) the underlying connection is never returned to the pool. GitLabs delete endpoints answer 200/202 with a body, so each such delete leaks one pooled connection; with the default of 2 connections per route the pool is quickly exhausted and subsequent requests block indefinitely in AbstractConnPool.getPoolEntryBlocking. This issue was not visible before #1312: setting client properties per request kept the ClientConfig RequestScoped, so the connector (and its pool) was rebuilt per request and any leaked connection was discarded with it. Since #1312 the ClientConfig is a singleton and the pool is shared, so the leak now accumulates and exhausts the pool. Buffer the delete body via Response.bufferEntity() in the delete() helpers. This releases the connection immediately while keeping the Response readable for the few callers that consume it (LicenseApi.deleteLicense, IssuesApi.deleteIssueLink, EpicsApi.removeIssue). get/post/put are unaffected since those callers already release the connection via readEntity(). --- .../java/org/gitlab4j/api/AbstractApi.java | 30 +++++++- .../gitlab4j/api/TestAbstractApiDelete.java | 73 +++++++++++++++++++ 2 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 gitlab4j-api/src/test/java/org/gitlab4j/api/TestAbstractApiDelete.java diff --git a/gitlab4j-api/src/main/java/org/gitlab4j/api/AbstractApi.java b/gitlab4j-api/src/main/java/org/gitlab4j/api/AbstractApi.java index 5ba97f25b..06441ed0e 100644 --- a/gitlab4j-api/src/main/java/org/gitlab4j/api/AbstractApi.java +++ b/gitlab4j-api/src/main/java/org/gitlab4j/api/AbstractApi.java @@ -696,7 +696,7 @@ protected Response delete( Response.Status expectedStatus, MultivaluedMap queryParams, Object... pathArgs) throws GitLabApiException { try { - return validate(getApiClient().delete(queryParams, pathArgs), expectedStatus); + return releaseConnection(validate(getApiClient().delete(queryParams, pathArgs), expectedStatus)); } catch (Exception e) { throw handle(e); } @@ -715,12 +715,38 @@ protected Response delete( protected Response delete(Response.Status expectedStatus, MultivaluedMap queryParams, URL url) throws GitLabApiException { try { - return validate(getApiClient().delete(queryParams, url), expectedStatus); + return releaseConnection(validate(getApiClient().delete(queryParams, url), expectedStatus)); } catch (Exception e) { throw handle(e); } } + /** + * Releases the underlying HTTP connection of a DELETE response back to the connection pool. + * + *

Most {@code delete(...)} callers (e.g. {@code GroupApi#deleteGroup}, + * {@code ProjectApi#deleteProject}) are {@code void} and discard the returned {@link Response} + * without closing it. When GitLabApi is configured with a pooling/Apache connector (for example + * via a proxy), a {@code Response} whose entity is never read or closed keeps its connection + * leased and never returns it to the pool. GitLab's delete endpoints typically answer 202/200 + * with a body, so these connections leak and the pool (default 2 per route) is + * exhausted, after which further requests block in {@code getPoolEntryBlocking}. + * + *

{@link Response#bufferEntity()} reads the body into memory and releases the + * connection immediately, while keeping the {@code Response} fully readable for the few callers + * that do consume the body (e.g. {@code LicenseApi#deleteLicense}, {@code IssuesApi#deleteIssueLink}, + * {@code EpicsApi#removeIssue}). + * + * @param response the validated DELETE response + * @return the same response, with its entity buffered and connection released + */ + private Response releaseConnection(Response response) { + if (response.hasEntity()) { + response.bufferEntity(); + } + return response; + } + /** * Convenience method for adding query and form parameters to a get() or post() call. * diff --git a/gitlab4j-api/src/test/java/org/gitlab4j/api/TestAbstractApiDelete.java b/gitlab4j-api/src/test/java/org/gitlab4j/api/TestAbstractApiDelete.java new file mode 100644 index 000000000..e5c6f952c --- /dev/null +++ b/gitlab4j-api/src/test/java/org/gitlab4j/api/TestAbstractApiDelete.java @@ -0,0 +1,73 @@ +package org.gitlab4j.api; + +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.nullable; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import jakarta.ws.rs.core.MultivaluedMap; +import jakarta.ws.rs.core.Response; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; + +/** + * Verifies that {@link AbstractApi#delete} releases its connection back to the pool. + * + *

Most delete callers are {@code void} and discard the returned {@link Response} without closing + * it; with a pooling/Apache connector that leaks the connection and eventually exhausts the pool + * (requests then block in {@code getPoolEntryBlocking}). The fix buffers the delete body, + * which releases the connection while keeping the response readable for callers that consume it. + */ +@ExtendWith(MockitoExtension.class) +public class TestAbstractApiDelete { + + private static class TestApi extends AbstractApi { + TestApi(GitLabApi gitLabApi) { + super(gitLabApi); + } + } + + private TestApi setupApi(Response response) throws Exception { + GitLabApi gitLabApi = mock(GitLabApi.class); + GitLabApiClient apiClient = mock(GitLabApiClient.class); + + when(gitLabApi.getApiClient()).thenReturn(apiClient); + when(apiClient.delete(nullable(MultivaluedMap.class), any(Object[].class))).thenReturn(response); + when(apiClient.validateSecretToken(response)).thenReturn(true); + when(response.getStatus()).thenReturn(Response.Status.OK.getStatusCode()); + + return new TestApi(gitLabApi); + } + + @Test + public void shouldBufferEntityToReleaseConnectionWhenBodyPresent() throws Exception { + Response response = mock(Response.class); + when(response.hasEntity()).thenReturn(true); + + TestApi api = setupApi(response); + Response result = api.delete(Response.Status.OK, null, "groups", 1L); + + assertSame(response, result); + + // The connection is released via bufferEntity() even though void callers discard the Response, + // and the Response stays readable (not closed) for callers that read the deleted entity. + verify(response).bufferEntity(); + verify(response, never()).close(); + } + + @Test + public void shouldNotBufferWhenNoEntity() throws Exception { + Response response = mock(Response.class); + when(response.hasEntity()).thenReturn(false); + + TestApi api = setupApi(response); + api.delete(Response.Status.OK, null, "groups", 1L); + + verify(response, never()).bufferEntity(); + } +}