Skip to content
17 changes: 14 additions & 3 deletions src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -339,12 +339,23 @@ public void clearHooks() {
* Once shut down is complete, API is reset and ready to use again.
*/
public void shutdown() {
List<FeatureProviderStateManager> managersToShutdown;
try (AutoCloseableLock ignored = lock.writeLockAutoCloseable()) {
providerRepository.shutdown();
// Mark repository as shutting down while holding lock.
// This ensures setProvider calls will throw IllegalStateException.
managersToShutdown = providerRepository.prepareShutdown();
}

if (managersToShutdown != null) {
// Complete shutdown without holding lock to avoid deadlock.
// Pending tasks (e.g., initializeProvider) may need the read lock to emit events.
providerRepository.completeShutdown(managersToShutdown);
eventSupport.shutdown();

providerRepository = new ProviderRepository(this);
eventSupport = new EventSupport();
try (AutoCloseableLock ignored = lock.writeLockAutoCloseable()) {
providerRepository = new ProviderRepository(this);
eventSupport = new EventSupport();
}
}
}

Expand Down
83 changes: 72 additions & 11 deletions src/main/java/dev/openfeature/sdk/ProviderRepository.java
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 @@ -162,6 +165,9 @@ private void prepareAndInitializeProvider(
final FeatureProviderStateManager oldStateManager;

synchronized (registerStateManagerLock) {
if (isShuttingDown.get()) {
throw new IllegalStateException("Provider cannot be set while repository is shutting down");
}
FeatureProviderStateManager existing = getExistingStateManagerForProvider(newProvider);
if (existing == null) {
newStateManager = new FeatureProviderStateManager(newProvider);
Expand Down Expand Up @@ -228,9 +234,11 @@ private void initializeProvider(
}

private void shutDownOld(FeatureProviderStateManager oldManager, Consumer<FeatureProvider> afterShutdown) {
if (oldManager != null && !isStateManagerRegistered(oldManager)) {
shutdownProvider(oldManager);
afterShutdown.accept(oldManager.getProvider());
synchronized (registerStateManagerLock) {
if (oldManager != null && !isStateManagerRegistered(oldManager)) {
shutdownProvider(oldManager);
afterShutdown.accept(oldManager.getProvider());
}
}
}

Expand All @@ -254,16 +262,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 +291,52 @@ private void shutdownProvider(FeatureProvider provider) {
* including the default feature provider.
*/
public void shutdown() {
Stream.concat(Stream.of(this.defaultStateManger.get()), this.stateManagers.values().stream())
.distinct()
.forEach(this::shutdownProvider);
this.stateManagers.clear();
List<FeatureProviderStateManager> managersToShutdown = prepareShutdown();
if (managersToShutdown != null) {
completeShutdown(managersToShutdown);
}
}

/**
* Prepares the repository for shutdown by marking it as shutting down and
* collecting all managers that need to be shut down.
*
* <p>After this call, any attempt to set a provider will throw IllegalStateException.
*
* @return list of managers to shut down, or null if shutdown was already initiated
*/
List<FeatureProviderStateManager> prepareShutdown() {
synchronized (registerStateManagerLock) {
if (isShuttingDown.getAndSet(true)) {
return null;
}

List<FeatureProviderStateManager> managersToShutdown = Stream.concat(
Stream.of(this.defaultStateManger.get()), this.stateManagers.values().stream())
.distinct()
.collect(Collectors.toList());
this.stateManagers.clear();
return managersToShutdown;
}
}

/**
* Completes the shutdown by shutting down all providers and waiting for
* pending tasks to complete.
*
* @param managersToShutdown the managers to shut down (from prepareShutdown)
*/
void completeShutdown(List<FeatureProviderStateManager> managersToShutdown) {
managersToShutdown.forEach(this::shutdownProvider);
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();
}
}
}
195 changes: 195 additions & 0 deletions src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java
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,199 @@ void shouldRunLambdasOnError() throws Exception {
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);
} 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());
}

// Note: shouldHandleInterruptionDuringShutdownGracefully was removed because the
// interrupt timing is not guaranteed. Proper concurrency testing is done in
// ProviderRepositoryCT using VMLens.

@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 shutdownCalled = new AtomicBoolean(false);

doDelayResponse(Duration.ofMillis(500)).when(slowInitProvider).initialize(any());

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

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

// 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(() -> setFeatureProvider(newProvider))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("shutting down");
}

@Test
@DisplayName("prepareShutdown should return null on second call")
void prepareShutdownShouldReturnNullOnSecondCall() {
FeatureProvider provider = createMockedProvider();
setFeatureProvider(provider);

// First call should return managers list
var managers = providerRepository.prepareShutdown();
assertThat(managers).isNotNull();
assertThat(managers).isNotEmpty();

// Second call should be a no-op and return null (already shutting down)
var secondResult = providerRepository.prepareShutdown();
assertThat(secondResult).isNull();
}

@Test
@DisplayName("should fall back to direct shutdown when executor rejects tasks")
void shouldFallBackToDirectShutdownWhenExecutorRejectsTasks() throws Exception {
FeatureProvider oldProvider = createMockedProvider();
FeatureProvider newProvider = createMockedProvider();
AtomicBoolean initializationStarted = new AtomicBoolean(false);
AtomicBoolean proceedWithInit = new AtomicBoolean(false);

// Make oldProvider's initialization block until we signal
doAnswer(invocation -> {
initializationStarted.set(true);
while (!proceedWithInit.get()) {
Thread.sleep(10);
}
return null;
})
.when(oldProvider)
.initialize(any());

// Start async initialization (will block)
providerRepository.setProvider(
oldProvider, mockAfterSet(), mockAfterInit(), mockAfterShutdown(), mockAfterError(), false);

// Wait for initialization to start
await().atMost(Duration.ofSeconds(1)).untilTrue(initializationStarted);

// Now set a new provider - this will trigger shutDownOld for oldProvider
// after initialization completes, but we haven't completed init yet
providerRepository.setProvider(
newProvider, mockAfterSet(), mockAfterInit(), mockAfterShutdown(), mockAfterError(), false);

// Call shutdown on repository - this will shutdown the executor
var managers = providerRepository.prepareShutdown();
providerRepository.completeShutdown(managers);

// Now let the initialization complete - shutDownOld will be called but executor is shutdown
// This triggers the RejectedExecutionException path which falls back to direct shutdown
proceedWithInit.set(true);

// Both providers should eventually be shut down (oldProvider via direct call due to
// RejectedExecutionException)
verify(oldProvider, timeout(TIMEOUT)).shutdown();
verify(newProvider, timeout(TIMEOUT)).shutdown();
}
}
}

@Test
Expand Down
14 changes: 14 additions & 0 deletions src/test/java/dev/openfeature/sdk/ShutdownBehaviorSpecTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -142,5 +142,19 @@ void apiIsReadyToUseAfterShutdown() {
NoOpProvider p2 = new NoOpProvider();
api.setProvider(p2);
}

@Test
@DisplayName("calling shutdown twice should be safe and idempotent")
void callingShutdownTwiceShouldBeSafe() {
FeatureProvider provider = ProviderFixture.createMockedProvider();
setFeatureProvider(provider);

api.shutdown();

// Second shutdown should be a no-op (no exception, provider not called twice)
api.shutdown();

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