Skip to content

fix: allow for providers to safely shutdown #1744

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: allow for providers to safely shutdown #1744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9efbc78
fix: allow for providers to safely shutdown
nicklasl Nov 25, 2025
dc340c1
fix: prevent race conditions during repository shutdown
nicklasl Nov 27, 2025
6ac560c
fixup! fix: prevent race conditions during repository shutdown
nicklasl Nov 27, 2025
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
39 changes: 35 additions & 4 deletions src/main/java/dev/openfeature/sdk/ProviderRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
Expand All @@ -23,6 +25,7 @@ class ProviderRepository {
private final Map<String, FeatureProviderStateManager> stateManagers = new ConcurrentHashMap<>();
private final AtomicReference<FeatureProviderStateManager> defaultStateManger =
new AtomicReference<>(new FeatureProviderStateManager(new NoOpProvider()));
private final AtomicBoolean isShuttingDown = new AtomicBoolean(false);
private final ExecutorService taskExecutor =
Executors.newCachedThreadPool(new ConfigurableThreadFactory("openfeature-provider-thread", true));
private final Object registerStateManagerLock = new Object();
Expand Down Expand Up @@ -158,6 +161,10 @@ private void prepareAndInitializeProvider(
Consumer<FeatureProvider> afterShutdown,
BiConsumer<FeatureProvider, OpenFeatureError> afterError,
boolean waitForInit) {
if (isShuttingDown.get()) {
throw new IllegalStateException("Provider cannot be set while repository is shutting down");
}

final FeatureProviderStateManager newStateManager;
final FeatureProviderStateManager oldStateManager;

Expand Down Expand Up @@ -254,16 +261,27 @@ private void shutdownProvider(FeatureProviderStateManager manager) {
}

private void shutdownProvider(FeatureProvider provider) {
taskExecutor.submit(() -> {
try {
taskExecutor.submit(() -> {
try {
provider.shutdown();
} catch (Exception e) {
log.error(
"Exception when shutting down feature provider {}",
provider.getClass().getName(),
e);
}
});
} catch (java.util.concurrent.RejectedExecutionException e) {
try {
provider.shutdown();
} catch (Exception e) {
} catch (Exception ex) {
log.error(
"Exception when shutting down feature provider {}",
provider.getClass().getName(),
e);
ex);
}
});
}
}

/**
Expand All @@ -272,10 +290,23 @@ private void shutdownProvider(FeatureProvider provider) {
* including the default feature provider.
*/
public void shutdown() {
if (isShuttingDown.getAndSet(true)) {
return;
}

Stream.concat(Stream.of(this.defaultStateManger.get()), this.stateManagers.values().stream())
.distinct()
.forEach(this::shutdownProvider);
this.stateManagers.clear();
taskExecutor.shutdown();
try {
if (!taskExecutor.awaitTermination(3, TimeUnit.SECONDS)) {
log.warn("Task executor did not terminate before the timeout period had elapsed");
taskExecutor.shutdownNow();
}
} catch (InterruptedException e) {
taskExecutor.shutdownNow();
Thread.currentThread().interrupt();
}
}
}
187 changes: 187 additions & 0 deletions src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static dev.openfeature.sdk.testutils.stubbing.ConditionStubber.doDelayResponse;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.awaitility.Awaitility.await;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
Expand All @@ -15,6 +16,7 @@
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
import java.util.function.Function;
Expand Down Expand Up @@ -289,6 +291,191 @@
verify(afterError, timeout(TIMEOUT)).accept(eq(errorFeatureProvider), any());
}
}

@Nested
class GracefulShutdownBehavior {

@Test
@DisplayName("should complete shutdown successfully when executor terminates within timeout")
void shouldCompleteShutdownSuccessfullyWhenExecutorTerminatesWithinTimeout() {
FeatureProvider provider = createMockedProvider();
setFeatureProvider(provider);

assertThatCode(() -> providerRepository.shutdown()).doesNotThrowAnyException();

verify(provider, timeout(TIMEOUT)).shutdown();
}

@Test
@DisplayName("should force shutdown when executor does not terminate within timeout")
void shouldForceShutdownWhenExecutorDoesNotTerminateWithinTimeout() throws Exception {
FeatureProvider provider = createMockedProvider();
AtomicBoolean wasInterrupted = new AtomicBoolean(false);
doAnswer(invocation -> {
try {
Thread.sleep(TIMEOUT);

Check warning on line 316 in src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this use of "Thread.sleep()". 


See more on https://sonarcloud.io/project/issues?id=open-feature_java-sdk&issues=AZq_LKeST-V0F1DO6NWd&open=AZq_LKeST-V0F1DO6NWd&pullRequest=1744
} catch (InterruptedException e) {
wasInterrupted.set(true);
throw e;
}
return null;
})
.when(provider)
.shutdown();

setFeatureProvider(provider);

assertThatCode(() -> providerRepository.shutdown()).doesNotThrowAnyException();

verify(provider, timeout(TIMEOUT)).shutdown();
// Verify that shutdownNow() interrupted the running shutdown task
await().atMost(Duration.ofSeconds(1))
.untilAsserted(() -> assertThat(wasInterrupted.get()).isTrue());
}

@Test
@DisplayName("should handle interruption during shutdown gracefully")
void shouldHandleInterruptionDuringShutdownGracefully() throws Exception {
FeatureProvider provider = createMockedProvider();
setFeatureProvider(provider);

Thread shutdownThread = new Thread(() -> {
providerRepository.shutdown();
Copy link
Contributor

@chrfwow chrfwow Nov 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

How long does the Mocked Provider take to shutdown? Are we sure we interrupt during this time?
Maybe this is a case for a VMLens test

});

shutdownThread.start();
shutdownThread.interrupt();
shutdownThread.join(TIMEOUT);

assertThat(shutdownThread.isAlive()).isFalse();
verify(provider, timeout(TIMEOUT)).shutdown();
}

@Test
@DisplayName("should not hang indefinitely on shutdown")
void shouldNotHangIndefinitelyOnShutdown() {
FeatureProvider provider = createMockedProvider();
setFeatureProvider(provider);

await().alias("shutdown should complete within reasonable time")
.atMost(Duration.ofSeconds(5))
.until(() -> {
providerRepository.shutdown();
return true;
});
}

@Test
@DisplayName("should handle shutdown during provider initialization")
void shouldHandleShutdownDuringProviderInitialization() throws Exception {
FeatureProvider slowInitProvider = createMockedProvider();
AtomicBoolean initStarted = new AtomicBoolean(false);
AtomicBoolean shutdownCalled = new AtomicBoolean(false);

doAnswer(invocation -> {
initStarted.set(true);
Thread.sleep(500);

Check warning on line 377 in src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this use of "Thread.sleep()". 


See more on https://sonarcloud.io/project/issues?id=open-feature_java-sdk&issues=AZrFT7UOi4UDv2I6dFJX&open=AZrFT7UOi4UDv2I6dFJX&pullRequest=1744
return null;
})
.when(slowInitProvider)
.initialize(any());

doAnswer(invocation -> {
shutdownCalled.set(true);
return null;
})
.when(slowInitProvider)
.shutdown();

providerRepository.setProvider(
slowInitProvider,
mockAfterSet(),
mockAfterInit(),
mockAfterShutdown(),
mockAfterError(),
false);

await().atMost(Duration.ofSeconds(1)).untilTrue(initStarted);

// Call shutdown while initialization is in progress
assertThatCode(() -> providerRepository.shutdown()).doesNotThrowAnyException();

await().atMost(Duration.ofSeconds(1)).untilTrue(shutdownCalled);
verify(slowInitProvider, times(1)).shutdown();
}

@Test
@DisplayName("should handle provider replacement during shutdown")
void shouldHandleProviderReplacementDuringShutdown() throws Exception {
FeatureProvider oldProvider = createMockedProvider();
FeatureProvider newProvider = createMockedProvider();
AtomicBoolean oldProviderShutdownCalled = new AtomicBoolean(false);

doAnswer(invocation -> {
oldProviderShutdownCalled.set(true);
return null;
})
.when(oldProvider)
.shutdown();

providerRepository.setProvider(
oldProvider, mockAfterSet(), mockAfterInit(), mockAfterShutdown(), mockAfterError(), true);

// Replace provider (this will trigger old provider shutdown in background)
providerRepository.setProvider(
newProvider, mockAfterSet(), mockAfterInit(), mockAfterShutdown(), mockAfterError(), false);

assertThatCode(() -> providerRepository.shutdown()).doesNotThrowAnyException();

await().atMost(Duration.ofSeconds(1)).untilTrue(oldProviderShutdownCalled);
verify(oldProvider, times(1)).shutdown();
verify(newProvider, times(1)).shutdown();
}

@Test
@DisplayName("should prevent adding providers after shutdown has started")
void shouldPreventAddingProvidersAfterShutdownHasStarted() {
FeatureProvider provider = createMockedProvider();
setFeatureProvider(provider);

providerRepository.shutdown();

FeatureProvider newProvider = createMockedProvider();
assertThatThrownBy(() -> providerRepository.setProvider(

Check warning on line 444 in src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor the code of the lambda to have only one invocation possibly throwing a runtime exception. 


See more on https://sonarcloud.io/project/issues?id=open-feature_java-sdk&issues=AZrFT7UOi4UDv2I6dFJY&open=AZrFT7UOi4UDv2I6dFJY&pullRequest=1744
newProvider,
mockAfterSet(),
mockAfterInit(),
mockAfterShutdown(),
mockAfterError(),
false))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("shutting down");
}

@Test
@DisplayName("should handle concurrent shutdown calls gracefully")
void shouldHandleConcurrentShutdownCallsGracefully() {
FeatureProvider provider = createMockedProvider();
setFeatureProvider(provider);

assertThatCode(() -> {
Thread t1 = new Thread(() -> providerRepository.shutdown());
Thread t2 = new Thread(() -> providerRepository.shutdown());
Thread t3 = new Thread(() -> providerRepository.shutdown());
Comment on lines +462 to +464
Copy link
Contributor

@chrfwow chrfwow Nov 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

We cannot be sure that any of these threads execute in parallel. This should be a VMLens test

Copy link
Member Author

@nicklasl nicklasl Dec 1, 2025

Choose a reason for hiding this comment

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

Thanks, I'll look into making them VMLens tests instead, just need to find some time :)


t1.start();
t2.start();
t3.start();

t1.join(TIMEOUT);
t2.join(TIMEOUT);
t3.join(TIMEOUT);
})
.doesNotThrowAnyException();

verify(provider, times(1)).shutdown();
}
}
}

@Test
Expand Down
Loading