Skip to content

Commit

Permalink
Harmonize closed state checks in connection managers.
Browse files Browse the repository at this point in the history
  • Loading branch information
arturobernalg committed Feb 21, 2024
1 parent 1480645 commit 6821955
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,11 @@ public void release(final ConnectionEndpoint endpoint, final Object state, final
if (LOG.isDebugEnabled()) {
LOG.debug("{} releasing endpoint", ConnPoolSupport.getId(endpoint));
}

if (this.isClosed()) {
return;
}

final ManagedHttpClientConnection conn = entry.getConnection();
if (conn != null && keepAlive == null) {
conn.close(CloseMode.GRACEFUL);
Expand Down Expand Up @@ -522,11 +527,17 @@ public void closeIdle(final TimeValue idleTime) {
if (LOG.isDebugEnabled()) {
LOG.debug("Closing connections idle longer than {}", idleTime);
}
if (isClosed()) {
return;
}
this.pool.closeIdle(idleTime);
}

@Override
public void closeExpired() {
if (isClosed()) {
return;
}
LOG.debug("Closing expired connections");
this.pool.closeExpired();
}
Expand Down Expand Up @@ -797,16 +808,13 @@ public EndpointInfo getInfo() {
}

/**
* Checks whether the {@link PoolingHttpClientConnectionManager} has been shut down.
* <p>
* This method returns {@code true} if the connection manager has been closed and is no longer accepting new connections.
* Once shut down, the connection manager cannot be used to manage HTTP connections.
* </p>
* Method that can be called to determine whether the connection manager has been shut down and
* is closed or not.
*
* @return {@code true} if this connection manager has been shut down, {@code false} otherwise.
* @since 5.4
* @return {@code true} if the connection manager has been shut down and is closed, otherwise
* return {@code false}.
*/
public boolean isShutdown() {
boolean isClosed() {
return this.closed.get();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,9 @@ public void release(final AsyncConnectionEndpoint endpoint, final Object state,
if (LOG.isDebugEnabled()) {
LOG.debug("{} releasing endpoint", ConnPoolSupport.getId(endpoint));
}
if (this.isClosed()) {
return;
}
final ManagedAsyncClientConnection connection = entry.getConnection();
boolean reusable = connection != null && connection.isOpen();
try {
Expand Down Expand Up @@ -575,11 +578,17 @@ public int getMaxPerRoute(final HttpRoute route) {

@Override
public void closeIdle(final TimeValue idletime) {
if (isClosed()) {
return;
}
pool.closeIdle(idletime);
}

@Override
public void closeExpired() {
if (isClosed()) {
return;
}
pool.closeExpired();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -355,15 +355,15 @@ public void testProxyConnectAndUpgrade() throws Exception {

@Test
public void testIsShutdownInitially() {
Assertions.assertFalse(mgr.isShutdown(), "Connection manager should not be shutdown initially.");
Assertions.assertFalse(mgr.isClosed(), "Connection manager should not be shutdown initially.");
}

@Test
public void testShutdownIdempotency() {
mgr.close();
Assertions.assertTrue(mgr.isShutdown(), "Connection manager should remain shutdown after the first call to shutdown.");
Assertions.assertTrue(mgr.isClosed(), "Connection manager should remain shutdown after the first call to shutdown.");
mgr.close(); // Second call to shutdown
Assertions.assertTrue(mgr.isShutdown(), "Connection manager should still be shutdown after subsequent calls to shutdown.");
Assertions.assertTrue(mgr.isClosed(), "Connection manager should still be shutdown after subsequent calls to shutdown.");
}

@Test
Expand All @@ -382,13 +382,13 @@ public void testIsShutdown() {
Mockito.when(pool.isShutdown()).thenReturn(false, true); // Simulate changing states

// Execution phase: Initially, the manager should not be shutdown
Assertions.assertFalse(mgr.isShutdown(), "Connection manager should not be shutdown initially.");
Assertions.assertFalse(mgr.isClosed(), "Connection manager should not be shutdown initially.");

// Simulate shutting down the manager
mgr.close();

// Verification phase: Now, the manager should be reported as shutdown
Assertions.assertTrue(mgr.isShutdown(), "Connection manager should be shutdown after close() is called.");
Assertions.assertTrue(mgr.isClosed(), "Connection manager should be shutdown after close() is called.");
}


Expand All @@ -401,7 +401,7 @@ public void testConcurrentShutdown() throws InterruptedException {
executor.shutdown();
executor.awaitTermination(1, TimeUnit.SECONDS);

Assertions.assertTrue(mgr.isShutdown(), "Connection manager should be shutdown after concurrent calls to shutdown.");
Assertions.assertTrue(mgr.isClosed(), "Connection manager should be shutdown after concurrent calls to shutdown.");
}


Expand Down

0 comments on commit 6821955

Please sign in to comment.