fix: allow for providers to safely shutdown #1744

nicklasl · 2025-11-26T07:58:50Z

Summary

Improves the shutdown behaviour of the ProviderRepository to ensure safe termination of the task executor. The previous implementation called shutdown() on the executor but did not wait for termination, potentially leading to abrupt shutdown, aborted provider shutdown sequences or hanging threads.

Changes

Added graceful shutdown handling in ProviderRepository.shutdown() with a 3-second timeout
Added shutdown state tracking to prevent race conditions
Handles RejectedExecutionException when tasks are submitted to a shut down executor

Implementation Details

The shutdown process now:

Calls shutdown() on the task executor to prevent new tasks
Waits up to 3 seconds for running tasks to complete via awaitTermination()
If tasks don't complete within the timeout, calls shutdownNow() to force termination
Properly handles InterruptedException by calling shutdownNow() and restoring the interrupt flag
Prevents new providers from being registered during shutdown via AtomicBoolean flag
Handles RejectedExecutionException by running provider shutdown synchronously when executor is closed

Test Coverage

Added comprehensive test coverage for shutdown behavior:
[x] Successful shutdown when executor terminates within timeout
[x] Forced shutdown when executor does not terminate within timeout
[x] Graceful handling of interruption during shutdown
[x] Protection against indefinite hangs during shutdown
[x] Concurrent shutdown calls (idempotent behavior)
[x] Shutdown during provider initialization
[x] Provider replacement during shutdown
[x] Prevention of new provider registration after shutdown starts

Related Issues

#1745

This change aligns with the broader effort to improve shutdown behaviour across the SDK, similar to PR #873 which addresses the same issue on Eventing.

codecov · 2025-11-26T08:14:45Z

Codecov Report

❌ Patch coverage is 81.81818% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.04%. Comparing base (3857856) to head (c83b7c4).

Files with missing lines	Patch %	Lines
...n/java/dev/openfeature/sdk/ProviderRepository.java	81.57%	5 Missing and 2 partials ⚠️
.../main/java/dev/openfeature/sdk/OpenFeatureAPI.java	83.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1744      +/-   ##
============================================
+ Coverage     92.51%   93.04%   +0.53%     
- Complexity      642      649       +7     
============================================
  Files            58       58              
  Lines          1536     1568      +32     
  Branches        170      176       +6     
============================================
+ Hits           1421     1459      +38     
+ Misses           70       63       -7     
- Partials         45       46       +1

Flag	Coverage Δ
unittests	`93.04% <81.81%> (+0.53%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

nicklasl · 2025-11-27T12:38:21Z

I added a commit to additionally prevent race conditions during shutdown:

Added AtomicBoolean isShuttingDown flag to track repository shutdown state
setProvider() now throws IllegalStateException if called during shutdown
Handles RejectedExecutionException when shutdown tasks try to submit to closed executor
Ensures idempotent shutdown (multiple concurrent calls are safe)

sonarqubecloud · 2025-11-27T12:41:04Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

chrfwow

The implementation looks nice, but the tests may have some race conditions

chrfwow · 2025-11-27T12:52:11Z

src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java

+                setFeatureProvider(provider);
+
+                Thread shutdownThread = new Thread(() -> {
+                    providerRepository.shutdown();


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

chrfwow · 2025-11-27T12:55:01Z

src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java

+                            Thread t1 = new Thread(() -> providerRepository.shutdown());
+                            Thread t2 = new Thread(() -> providerRepository.shutdown());
+                            Thread t3 = new Thread(() -> providerRepository.shutdown());


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

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

nicklasl · 2026-01-26T09:09:28Z

@chrfwow I've been working on the shutdown behavior fix for ProviderRepository a bit on the side and ran into a VMLens issue I could use help with.

I wrote a CT test (ProviderRepositoryCT) to verify concurrent shutdown behavior, but VMLens crashes when ThreadPoolExecutor.shutdown() is called inside AllInterleavings:

  NullPointerException: Cannot invoke "TLinkedList.iterator()" because "tasks" is null
    at com.vmlens...ThreadPoolMap.joinAll()
    at java.util.concurrent.ThreadPoolExecutor.shutdown()
    at dev.openfeature.sdk.EventSupport.shutdown()

Claude tells me that VMLens instruments ThreadPoolExecutor and its internal state gets corrupted when shutdown() runs during test execution. The existing VmLensCT avoids this by calling shutdown() in @AfterEach (outside AllInterleavings), but that defeats the purpose of testing concurrent shutdown.

I've commented out the tests for now but I'm wondering if you've encountered this before? Any ideas for testing concurrent shutdown with VMLens, or should we rely on the regular unit tests?

chrfwow · 2026-01-26T08:58:03Z

src/main/java/dev/openfeature/sdk/ProviderRepository.java

+
        Stream.concat(Stream.of(this.defaultStateManger.get()), this.stateManagers.values().stream())
                .distinct()
                .forEach(this::shutdownProvider);
        this.stateManagers.clear();
        taskExecutor.shutdown();


I wonder if we have to wrap this with synchronized (registerStateManagerLock) {, otherwise we might lose additions to this.stateManagers

src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java

chrfwow · 2026-01-26T13:43:43Z

@nicklasl I opened an issue for the NPE in VmLens: vmlens/vmlens#58
If it is not too much work, I would keep the old tests that you had before VmLens, and update to the current version as soon as the issue is fixed. Speaking from experience, VmLens bugs are typically fixed quickly.

chrfwow · 2026-02-09T07:14:03Z

@nicklasl the bug in the VmLens Library should now be fixed. Could you please re-enable the VMLens tests?

Signed-off-by: Nicklas Lundin <nicklasl@spotify.com>

VMLens crashes with NPE when ThreadPoolExecutor.shutdown() is called inside AllInterleavings block. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Nicklas Lundin <nicklasl@spotify.com>

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Nicklas Lundin <nicklasl@spotify.com>

Split ProviderRepository.shutdown() into prepareShutdown() and completeShutdown() phases. OpenFeatureAPI.shutdown() now releases the write lock before waiting for executor termination, allowing pending initializeProvider tasks to acquire read lock for event emission. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Nicklas Lundin <nicklasl@spotify.com>

Reduce from 3 to 2 providers to work around VMLens graph building bug. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Nicklas Lundin <nicklasl@spotify.com>

chrfwow

Approving, but please consider my comment

chrfwow · 2026-02-09T13:35:49Z

src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java

+                OpenFeatureAPI api = OpenFeatureAPITestUtil.createAPI();
+
+                // Set provider and wait for initialization to complete
+                api.setProviderAndWait(provider);


Would it make sense to copy this test but to call setProvider (Without wait) instead in the copy?

Good point. I added an additional test with this 👍

Signed-off-by: Nicklas Lundin <nicklasl@spotify.com>

sonarqubecloud · 2026-02-09T13:59:34Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

nicklasl force-pushed the fix-shutdown-behaviour branch from bb09ece to 08e5fc7 Compare November 26, 2025 07:59

toddbaert requested review from aepfli, chrfwow, lopitz and toddbaert November 26, 2025 16:54

aepfli approved these changes Nov 27, 2025

View reviewed changes

nicklasl force-pushed the fix-shutdown-behaviour branch from 7271a12 to dc340c1 Compare November 27, 2025 12:35

nicklasl marked this pull request as ready for review November 27, 2025 12:42

nicklasl requested review from a team as code owners November 27, 2025 12:42

chrfwow reviewed Nov 27, 2025

View reviewed changes

nicklasl force-pushed the fix-shutdown-behaviour branch from f612687 to 5fe4cbc Compare January 26, 2026 07:02

nicklasl force-pushed the fix-shutdown-behaviour branch 3 times, most recently from 4c6709a to eefdb14 Compare January 26, 2026 09:11

chrfwow reviewed Jan 26, 2026

View reviewed changes

chrfwow mentioned this pull request Jan 26, 2026

NPE when shutting down a ThreadPoolExecutor during a test vmlens/vmlens#58

Closed

nicklasl force-pushed the fix-shutdown-behaviour branch from e3325f9 to 2ac107b Compare January 27, 2026 15:23

nicklasl force-pushed the fix-shutdown-behaviour branch 3 times, most recently from cf255c9 to efb3475 Compare February 6, 2026 15:19

nicklasl force-pushed the fix-shutdown-behaviour branch from efb3475 to 0cb88d6 Compare February 9, 2026 08:21

nicklasl added 2 commits February 9, 2026 09:23

fix: allow for providers to safely shutdown

ad34c63

Signed-off-by: Nicklas Lundin <nicklasl@spotify.com>

fix: prevent race conditions during repository shutdown

7efe5fd

Signed-off-by: Nicklas Lundin <nicklasl@spotify.com>

nicklasl and others added 2 commits February 9, 2026 09:23

test: comment out ProviderRepositoryCT due to VMLens limitation

09f1ea1

VMLens crashes with NPE when ThreadPoolExecutor.shutdown() is called inside AllInterleavings block. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Nicklas Lundin <nicklasl@spotify.com>

refactor: use throws declaration instead of try-catch in test helper

d3078b2

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Nicklas Lundin <nicklasl@spotify.com>

nicklasl force-pushed the fix-shutdown-behaviour branch from 0cb88d6 to 675132e Compare February 9, 2026 08:23

nicklasl force-pushed the fix-shutdown-behaviour branch from 675132e to f925089 Compare February 9, 2026 08:26

test: simplify VMLens concurrency test for multiple providers

27247bf

Reduce from 3 to 2 providers to work around VMLens graph building bug. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Nicklas Lundin <nicklasl@spotify.com>

nicklasl force-pushed the fix-shutdown-behaviour branch from f925089 to 27247bf Compare February 9, 2026 09:30

chrfwow approved these changes Feb 9, 2026

View reviewed changes

additional test for setProvider and shutdown

c83b7c4

Signed-off-by: Nicklas Lundin <nicklasl@spotify.com>

nicklasl force-pushed the fix-shutdown-behaviour branch from 87acf1a to c83b7c4 Compare February 9, 2026 13:58

fix: allow for providers to safely shutdown #1744

Are you sure you want to change the base?

fix: allow for providers to safely shutdown #1744

Conversation

nicklasl commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Implementation Details

Test Coverage

Related Issues

Uh oh!

codecov bot commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nicklasl commented Nov 27, 2025

Uh oh!

sonarqubecloud bot commented Nov 27, 2025

Quality Gate passed

Uh oh!

chrfwow left a comment

Choose a reason for hiding this comment

Uh oh!

chrfwow Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrfwow Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicklasl Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

nicklasl commented Jan 26, 2026

Uh oh!

chrfwow Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

chrfwow commented Jan 26, 2026

Uh oh!

chrfwow commented Feb 9, 2026

Uh oh!

chrfwow left a comment

Choose a reason for hiding this comment

Uh oh!

chrfwow Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

nicklasl Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Feb 9, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nicklasl commented Nov 26, 2025 •

edited

Loading

codecov bot commented Nov 26, 2025 •

edited

Loading

chrfwow Nov 27, 2025 •

edited

Loading

chrfwow Nov 27, 2025 •

edited

Loading