feat: add means of awaiting event emission, fix flaky build #1463

chrfwow · 2025-05-28T12:04:24Z

This PR

adds the ability to await event emissions by returning a new construct
uses this construct in tests

⚠️ in rare cases, this could be a breaking change, but the events API is currently experimental, so we will not do a major version ubmp

Related Issues

Fixes #1449

Signed-off-by: christian.lutnik <[email protected]>

chrfwow · 2025-05-28T12:08:54Z

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

-        final TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> localOnEmit = this.onEmit;
-        if (localOnEmit != null) {
-            emitterExecutor.submit(() -> localOnEmit.accept(this, event, details));
+        try (var ignored = OpenFeatureAPI.lock.readLockAutoCloseable()) {


It's not nice to access the static lock that's clearly only exposed for testing, but we need to use this very lock to protect against concurrent reads/writes to the underlying map.

I removed the call to the emitterExecutor, so now all listeners are invoked sequentially on the calling thread. This was needed as several tests expect the state of the provider to be set immediately after calling this method.
If we consider this a problem, I could work around this issue by returning something our tests can wait for. In this case, we would need to call both the localOnEmit and the localEventProviderListener in the emitterExecutor.

By moving the call from the emitterExecutor to the current thread, I reintroduced a race condition that was already fixed.
Therefore I added the executor again. This means that some tests will fail that expect the state of the provider to be updated immediately after a call to emit. This is why I now return an Awaitable, for which those tests (or users) can wait.

Signed-off-by: christian.lutnik <[email protected]>

chrfwow · 2025-05-28T12:10:10Z

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

@@ -21,7 +21,6 @@
 @Slf4j
 public abstract class EventProvider implements FeatureProvider {
    private EventProviderListener eventProviderListener;
-    private final ExecutorService emitterExecutor = Executors.newCachedThreadPool();


Not needed anymore

Wrong, we do need it

Signed-off-by: christian.lutnik <[email protected]>

aepfli

Thank you! I really would love to have the same detail and knowledge as you have for concurrency. Thank you!

Signed-off-by: christian.lutnik <[email protected]>

codecov · 2025-06-03T06:29:44Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.40%. Comparing base (6cca721) to head (0228a70).
Report is 1 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1463      +/-   ##
============================================
+ Coverage     92.88%   93.40%   +0.51%     
- Complexity      483      490       +7     
============================================
  Files            45       46       +1     
  Lines          1167     1182      +15     
  Branches        101      103       +2     
============================================
+ Hits           1084     1104      +20     
+ Misses           53       48       -5     
  Partials         30       30

Flag	Coverage Δ
unittests	`93.40% <100.00%> (+0.51%)`	⬆️

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.

chrfwow · 2025-06-03T08:01:02Z

@toddbaert in theory, this is a breaking change. The state of the provider is not set synchronously when calling the emit() method, but only after the emitterExecutor runs. Users can wait for the state change to be propagated by calling await() on the return value of emit().

toddbaert · 2025-06-09T13:31:20Z

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

+/**
+ * A class to help with synchronization.
+ */
+public class Awaitable {


I like this.

toddbaert · 2025-06-09T13:34:18Z

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

-            handlerMap.put(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, new ArrayList<>());
-            handlerMap.put(ProviderEvent.PROVIDER_ERROR, new ArrayList<>());
-            handlerMap.put(ProviderEvent.PROVIDER_STALE, new ArrayList<>());
+            handlerMap.put(ProviderEvent.PROVIDER_READY, new ConcurrentLinkedQueue<>());


toddbaert · 2025-06-09T14:08:23Z

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

@@ -93,8 +110,8 @@ public void emit(ProviderEvent event, ProviderEventDetails details) {
     *
     * @param details The details of the event
     */
-    public void emitProviderReady(ProviderEventDetails details) {
-        emit(ProviderEvent.PROVIDER_READY, details);
+    public Awaitable emitProviderReady(ProviderEventDetails details) {


This is a public class people can override to implement a provider that supports events. I like this API and I think it's handy to have.

I was trying to see if there was any way this could be considered breaking. I tried adding this at the top of the class:

private BiConsumer<ProviderEvent, ProviderEventDetails> doesThisCompile = this::emit;

And (surprisingly to me) before and after your change, this still compiles even though BIcConsumer returns void. Apparently, return values of method references are ignored when assigned to void functional interfaces!

EDIT:

Oh - I just realized, this could break implementations that override these methods... I think that would be relatively rare but it's possible. What do you think @aepfli @chrfwow ?

I don't think this would be much of a problem. Providers can just return Awaitable.FINISHED if they do this synchronously, otherwise it's probably best to return something to get informed about the state update anyway.

…pen-feature#1449

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

Signed-off-by: Todd Baert <[email protected]>

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

Signed-off-by: Todd Baert <[email protected]>

sonarqubecloud · 2025-06-10T15:20:22Z

Quality Gate passed

Issues
1 New issue
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

Flaky build due to a possible race condition open-feature#1449

306b74f

Signed-off-by: christian.lutnik <[email protected]>

chrfwow requested review from a team as code owners May 28, 2025 12:04

chrfwow commented May 28, 2025

View reviewed changes

Flaky build due to a possible race condition open-feature#1449

5bd4586

Signed-off-by: christian.lutnik <[email protected]>

chrfwow commented May 28, 2025

View reviewed changes

Flaky build due to a possible race condition open-feature#1449

caae39d

Signed-off-by: christian.lutnik <[email protected]>

aepfli approved these changes Jun 2, 2025

View reviewed changes

Flaky build due to a possible race condition open-feature#1449

5909f33

Signed-off-by: christian.lutnik <[email protected]>

chrfwow requested a review from aepfli June 2, 2025 13:02

Flaky build due to a possible race condition open-feature#1449

43c06f3

Signed-off-by: christian.lutnik <[email protected]>

chrfwow requested a review from a team June 2, 2025 13:03

chrfwow added 8 commits June 2, 2025 15:06

Flaky build due to a possible race condition open-feature#1449

9e2cbd8

Signed-off-by: christian.lutnik <[email protected]>

Flaky build due to a possible race condition open-feature#1449

694526e

Signed-off-by: christian.lutnik <[email protected]>

Flaky build due to a possible race condition open-feature#1449

95d2271

Signed-off-by: christian.lutnik <[email protected]>

Flaky build due to a possible race condition open-feature#1449

0738f9f

Signed-off-by: christian.lutnik <[email protected]>

Flaky build due to a possible race condition open-feature#1449

3a1ce09

Signed-off-by: christian.lutnik <[email protected]>

Flaky build due to a possible race condition open-feature#1449

5ab1f68

Signed-off-by: christian.lutnik <[email protected]>

Flaky build due to a possible race condition open-feature#1449

1feacdd

Signed-off-by: christian.lutnik <[email protected]>

Flaky build due to a possible race condition open-feature#1449

6a4fa3d

Signed-off-by: christian.lutnik <[email protected]>

aepfli approved these changes Jun 3, 2025

View reviewed changes

chrfwow requested a review from a team June 4, 2025 14:55

aepfli approved these changes Jun 5, 2025

View reviewed changes

chrfwow requested a review from toddbaert June 5, 2025 08:15

alexandraoberaigner approved these changes Jun 6, 2025

View reviewed changes

toddbaert reviewed Jun 9, 2025

View reviewed changes

toddbaert approved these changes Jun 9, 2025

View reviewed changes

Merge branch 'main' into Flaky-build-due-to-a-possible-race-condition-o…

c22da3e

…pen-feature#1449

toddbaert changed the title ~~fix: Flaky build due to a possible race condition~~ feat: add means of awaiting event emission, fix flaky build Jun 10, 2025

Merge branch 'main' into Flaky-build-due-to-a-possible-race-condition-o…

14d8c12

…pen-feature#1449

toddbaert reviewed Jun 10, 2025

View reviewed changes

src/main/java/dev/openfeature/sdk/Awaitable.java Outdated Show resolved Hide resolved

Update src/main/java/dev/openfeature/sdk/Awaitable.java

596fe22

Signed-off-by: Todd Baert <[email protected]>

toddbaert reviewed Jun 10, 2025

View reviewed changes

src/main/java/dev/openfeature/sdk/Awaitable.java Show resolved Hide resolved

Update src/main/java/dev/openfeature/sdk/Awaitable.java

0228a70

Signed-off-by: Todd Baert <[email protected]>

toddbaert merged commit 3dd7d5d into open-feature:main Jun 10, 2025
13 checks passed

openfeaturebot mentioned this pull request Jun 10, 2025

chore(main): release 1.16.0 #1452

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: add means of awaiting event emission, fix flaky build #1463

feat: add means of awaiting event emission, fix flaky build #1463

Uh oh!

chrfwow commented May 28, 2025 •

edited by toddbaert

Loading

Uh oh!

chrfwow May 28, 2025

Uh oh!

chrfwow Jun 2, 2025

Uh oh!

chrfwow May 28, 2025

Uh oh!

chrfwow Jun 2, 2025

Uh oh!

aepfli left a comment

Uh oh!

codecov bot commented Jun 3, 2025 •

edited

Loading

Uh oh!

chrfwow commented Jun 3, 2025

Uh oh!

toddbaert Jun 9, 2025

Uh oh!

toddbaert Jun 9, 2025

Uh oh!

toddbaert Jun 9, 2025 •

edited

Loading

Uh oh!

chrfwow Jun 10, 2025

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Jun 10, 2025

Uh oh!

Uh oh!

Uh oh!

feat: add means of awaiting event emission, fix flaky build #1463

feat: add means of awaiting event emission, fix flaky build #1463

Uh oh!

Conversation

chrfwow commented May 28, 2025 • edited by toddbaert Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This PR

Related Issues

Uh oh!

chrfwow May 28, 2025

Choose a reason for hiding this comment

Uh oh!

chrfwow Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

chrfwow May 28, 2025

Choose a reason for hiding this comment

Uh oh!

chrfwow Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

aepfli left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

chrfwow commented Jun 3, 2025

Uh oh!

toddbaert Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

toddbaert Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

toddbaert Jun 9, 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 Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Jun 10, 2025

Quality Gate passed

Uh oh!

Uh oh!

Uh oh!

chrfwow commented May 28, 2025 •

edited by toddbaert

Loading

codecov bot commented Jun 3, 2025 •

edited

Loading

toddbaert Jun 9, 2025 •

edited

Loading