diff --git a/core/src/main/java/com/linecorp/armeria/client/endpoint/healthcheck/HealthCheckedEndpointGroup.java b/core/src/main/java/com/linecorp/armeria/client/endpoint/healthcheck/HealthCheckedEndpointGroup.java index 637da21fe30..ea56d7b719e 100644 --- a/core/src/main/java/com/linecorp/armeria/client/endpoint/healthcheck/HealthCheckedEndpointGroup.java +++ b/core/src/main/java/com/linecorp/armeria/client/endpoint/healthcheck/HealthCheckedEndpointGroup.java @@ -194,7 +194,7 @@ private void setCandidates(List endpoints) { } initialized = true; destroyOldContexts(contextGroup); - setEndpoints(allHealthyEndpoints()); + setEndpoints0(allHealthyEndpoints()); return null; }); } finally { @@ -291,6 +291,12 @@ private void destroyOldContexts(HealthCheckContextGroup contextGroup) { } private void updateHealth(Endpoint endpoint, boolean health) { + if (isClosing()) { + logger.debug("HealthCheckedEndpointGroup is closed. Ignoring health update for: {}. (healthy: {})", + endpoint, health); + return; + } + final boolean updated; // A healthy endpoint should be a valid checker context. if (health && findContext(endpoint) != null) { @@ -303,8 +309,15 @@ private void updateHealth(Endpoint endpoint, boolean health) { // Each new health status will be updated after initialization of the first context group. if (updated && initialized) { - setEndpoints(allHealthyEndpoints()); + setEndpoints0(allHealthyEndpoints()); + } + } + + private void setEndpoints0(List endpoints) { + if (isClosing()) { + return; } + setEndpoints(endpoints); } @Override diff --git a/core/src/test/java/com/linecorp/armeria/client/endpoint/healthcheck/HealthCheckedEndpointGroupTest.java b/core/src/test/java/com/linecorp/armeria/client/endpoint/healthcheck/HealthCheckedEndpointGroupTest.java index 2b0452c5434..f6ee0f93b2f 100644 --- a/core/src/test/java/com/linecorp/armeria/client/endpoint/healthcheck/HealthCheckedEndpointGroupTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/endpoint/healthcheck/HealthCheckedEndpointGroupTest.java @@ -602,6 +602,38 @@ void cacheReflectsAttributeChanges() throws InterruptedException { } } + @Test + void shouldStopUpdatingEndpointsWhenClosing() throws InterruptedException { + final AtomicInteger counter = new AtomicInteger(); + final Function checkFactory = ctx -> { + counter.incrementAndGet(); + ctx.updateHealth(HEALTHY, null, null, null); + return AsyncCloseableSupport.of(); + }; + + final Endpoint candidate1 = Endpoint.of("candidate1"); + final Endpoint candidate2 = Endpoint.of("candidate2"); + + final MockEndpointGroup delegate = new MockEndpointGroup(); + delegate.set(candidate1, candidate2, candidate2); + + final HealthCheckedEndpointGroup endpointGroup = + new HealthCheckedEndpointGroup(delegate, true, + 10000, 10000, + SessionProtocol.HTTP, 80, + DEFAULT_HEALTH_CHECK_RETRY_BACKOFF, + ClientOptions.of(), checkFactory, + HealthCheckStrategy.all(), + DEFAULT_ENDPOINT_PREDICATE); + assertThat(counter.get()).isEqualTo(2); + final EndpointComparator comparator = new EndpointComparator(); + assertThat(endpointGroup.endpoints()).usingElementComparator(comparator) + .containsOnly(candidate1, candidate2); + endpointGroup.close(); + assertThat(endpointGroup.endpoints()).usingElementComparator(comparator) + .containsOnly(candidate1, candidate2); + } + static final class MockEndpointGroup extends DynamicEndpointGroup { MockEndpointGroup() {}