From 306b74f5d16c5c3c3e63d8257a75db52e2ed3930 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Wed, 28 May 2025 14:03:42 +0200 Subject: [PATCH 01/15] Flaky build due to a possible race condition #1449 Signed-off-by: christian.lutnik --- .../dev/openfeature/sdk/EventProvider.java | 19 +++++++++++++------ .../java/dev/openfeature/sdk/EventsTest.java | 8 ++++---- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index 659c6ad46..512b00b81 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -76,14 +76,21 @@ public void shutdown() { * @param event The event type * @param details The details of the event */ - public void emit(ProviderEvent event, ProviderEventDetails details) { - if (eventProviderListener != null) { - eventProviderListener.onEmit(event, details); + public void emit(final ProviderEvent event, final ProviderEventDetails details) { + final var localEventProviderListener = this.eventProviderListener; + final var localOnEmit = this.onEmit; + + if (localEventProviderListener == null && localOnEmit == null) { + return; } - final TriConsumer localOnEmit = this.onEmit; - if (localOnEmit != null) { - emitterExecutor.submit(() -> localOnEmit.accept(this, event, details)); + try (var ignored = OpenFeatureAPI.lock.readLockAutoCloseable()) { + if (localEventProviderListener != null) { + localEventProviderListener.onEmit(event, details); + } + if (localOnEmit != null) { + localOnEmit.accept(this, event, details); + } } } diff --git a/src/test/java/dev/openfeature/sdk/EventsTest.java b/src/test/java/dev/openfeature/sdk/EventsTest.java index 157c0bafe..08c067f6e 100644 --- a/src/test/java/dev/openfeature/sdk/EventsTest.java +++ b/src/test/java/dev/openfeature/sdk/EventsTest.java @@ -24,7 +24,7 @@ class EventsTest { private OpenFeatureAPI api; @BeforeEach - public void setUp() throws Exception { + void setUp() { api = new OpenFeatureAPI(); } @@ -578,7 +578,7 @@ void shouldHaveAllProperties() { number = "5.3.3", text = "Handlers attached after the provider is already in the associated state, MUST run immediately.") void matchingReadyEventsMustRunImmediately() { - final String name = "matchingEventsMustRunImmediately"; + final String name = "matchingReadyEventsMustRunImmediately"; final Consumer handler = mockHandler(); // provider which is already ready @@ -597,7 +597,7 @@ void matchingReadyEventsMustRunImmediately() { number = "5.3.3", text = "Handlers attached after the provider is already in the associated state, MUST run immediately.") void matchingStaleEventsMustRunImmediately() { - final String name = "matchingEventsMustRunImmediately"; + final String name = "matchingStaleEventsMustRunImmediately"; final Consumer handler = mockHandler(); // provider which is already stale @@ -618,7 +618,7 @@ void matchingStaleEventsMustRunImmediately() { number = "5.3.3", text = "Handlers attached after the provider is already in the associated state, MUST run immediately.") void matchingErrorEventsMustRunImmediately() { - final String name = "matchingEventsMustRunImmediately"; + final String name = "matchingErrorEventsMustRunImmediately"; final Consumer handler = mockHandler(); // provider which is already in error From 5bd45867c9c163d04c2d830213f4e4e38c3e9371 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Wed, 28 May 2025 14:09:55 +0200 Subject: [PATCH 02/15] Flaky build due to a possible race condition #1449 Signed-off-by: christian.lutnik --- src/main/java/dev/openfeature/sdk/EventProvider.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index 512b00b81..33e8e8996 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/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(); void setEventProviderListener(EventProviderListener eventProviderListener) { this.eventProviderListener = eventProviderListener; @@ -58,16 +57,6 @@ void detach() { */ @Override public void shutdown() { - emitterExecutor.shutdown(); - try { - if (!emitterExecutor.awaitTermination(EventSupport.SHUTDOWN_TIMEOUT_SECONDS, TimeUnit.SECONDS)) { - log.warn("Emitter executor did not terminate before the timeout period had elapsed"); - emitterExecutor.shutdownNow(); - } - } catch (InterruptedException e) { - emitterExecutor.shutdownNow(); - Thread.currentThread().interrupt(); - } } /** From caae39d64b6ccd3794a0990ad0f5652c3c410236 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Wed, 28 May 2025 14:18:27 +0200 Subject: [PATCH 03/15] Flaky build due to a possible race condition #1449 Signed-off-by: christian.lutnik --- src/main/java/dev/openfeature/sdk/EventProvider.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index 33e8e8996..064348f38 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -1,9 +1,6 @@ package dev.openfeature.sdk; import dev.openfeature.sdk.internal.TriConsumer; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.TimeUnit; import lombok.extern.slf4j.Slf4j; /** @@ -56,8 +53,7 @@ void detach() { * or timeout period has elapsed. */ @Override - public void shutdown() { - } + public void shutdown() {} /** * Emit the specified {@link ProviderEvent}. From 5909f33ac35c131f007f24c2175dffe73958af30 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Mon, 2 Jun 2025 14:42:48 +0200 Subject: [PATCH 04/15] Flaky build due to a possible race condition #1449 Signed-off-by: christian.lutnik --- .../java/dev/openfeature/sdk/Awaitable.java | 32 ++++++++++ .../dev/openfeature/sdk/EventProvider.java | 59 +++++++++++++------ .../dev/openfeature/sdk/EventSupport.java | 28 ++++----- .../sdk/DeveloperExperienceTest.java | 8 +-- .../openfeature/sdk/EventProviderTest.java | 1 + .../java/dev/openfeature/sdk/EventsTest.java | 4 +- .../memory/InMemoryProviderTest.java | 18 +++++- 7 files changed, 107 insertions(+), 43 deletions(-) create mode 100644 src/main/java/dev/openfeature/sdk/Awaitable.java diff --git a/src/main/java/dev/openfeature/sdk/Awaitable.java b/src/main/java/dev/openfeature/sdk/Awaitable.java new file mode 100644 index 000000000..f5ea45366 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/Awaitable.java @@ -0,0 +1,32 @@ +package dev.openfeature.sdk; + +public class Awaitable { + public static final Awaitable FINISHED = new Awaitable(true); + + private boolean isDone = false; + + public Awaitable() {} + + private Awaitable(boolean isDone) { + this.isDone = isDone; + } + + public void await() { + if (isDone) { + return; + } + synchronized (this) { + while (!isDone) { + try { + this.wait(); + } catch (InterruptedException ignored) { + } + } + } + } + + public synchronized void wakeup() { + isDone = true; + this.notifyAll(); + } +} diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index 064348f38..dc28ee5ae 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -1,6 +1,9 @@ package dev.openfeature.sdk; import dev.openfeature.sdk.internal.TriConsumer; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import lombok.extern.slf4j.Slf4j; /** @@ -18,6 +21,7 @@ @Slf4j public abstract class EventProvider implements FeatureProvider { private EventProviderListener eventProviderListener; + private final ExecutorService emitterExecutor = Executors.newCachedThreadPool(); void setEventProviderListener(EventProviderListener eventProviderListener) { this.eventProviderListener = eventProviderListener; @@ -53,7 +57,18 @@ void detach() { * or timeout period has elapsed. */ @Override - public void shutdown() {} + public void shutdown() { + emitterExecutor.shutdown(); + try { + if (!emitterExecutor.awaitTermination(EventSupport.SHUTDOWN_TIMEOUT_SECONDS, TimeUnit.SECONDS)) { + log.warn("Emitter executor did not terminate before the timeout period had elapsed"); + emitterExecutor.shutdownNow(); + } + } catch (InterruptedException e) { + emitterExecutor.shutdownNow(); + Thread.currentThread().interrupt(); + } + } /** * Emit the specified {@link ProviderEvent}. @@ -61,22 +76,30 @@ public void shutdown() {} * @param event The event type * @param details The details of the event */ - public void emit(final ProviderEvent event, final ProviderEventDetails details) { + public Awaitable emit(final ProviderEvent event, final ProviderEventDetails details) { final var localEventProviderListener = this.eventProviderListener; final var localOnEmit = this.onEmit; if (localEventProviderListener == null && localOnEmit == null) { - return; + return Awaitable.FINISHED; } - try (var ignored = OpenFeatureAPI.lock.readLockAutoCloseable()) { - if (localEventProviderListener != null) { - localEventProviderListener.onEmit(event, details); - } - if (localOnEmit != null) { - localOnEmit.accept(this, event, details); + final var awaitable = new Awaitable(); + + emitterExecutor.submit(() -> { + try (var ignored = OpenFeatureAPI.lock.readLockAutoCloseable()) { + if (localEventProviderListener != null) { + localEventProviderListener.onEmit(event, details); + } + if (localOnEmit != null) { + localOnEmit.accept(this, event, details); + } + } finally { + awaitable.wakeup(); } - } + }); + + return awaitable; } /** @@ -85,8 +108,8 @@ public void emit(final ProviderEvent event, final ProviderEventDetails details) * * @param details The details of the event */ - public void emitProviderReady(ProviderEventDetails details) { - emit(ProviderEvent.PROVIDER_READY, details); + public Awaitable emitProviderReady(ProviderEventDetails details) { + return emit(ProviderEvent.PROVIDER_READY, details); } /** @@ -96,8 +119,8 @@ public void emitProviderReady(ProviderEventDetails details) { * * @param details The details of the event */ - public void emitProviderConfigurationChanged(ProviderEventDetails details) { - emit(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, details); + public Awaitable emitProviderConfigurationChanged(ProviderEventDetails details) { + return emit(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, details); } /** @@ -106,8 +129,8 @@ public void emitProviderConfigurationChanged(ProviderEventDetails details) { * * @param details The details of the event */ - public void emitProviderStale(ProviderEventDetails details) { - emit(ProviderEvent.PROVIDER_STALE, details); + public Awaitable emitProviderStale(ProviderEventDetails details) { + return emit(ProviderEvent.PROVIDER_STALE, details); } /** @@ -116,7 +139,7 @@ public void emitProviderStale(ProviderEventDetails details) { * * @param details The details of the event */ - public void emitProviderError(ProviderEventDetails details) { - emit(ProviderEvent.PROVIDER_ERROR, details); + public Awaitable emitProviderError(ProviderEventDetails details) { + return emit(ProviderEvent.PROVIDER_ERROR, details); } } diff --git a/src/main/java/dev/openfeature/sdk/EventSupport.java b/src/main/java/dev/openfeature/sdk/EventSupport.java index 5ebe90a4c..8396795bd 100644 --- a/src/main/java/dev/openfeature/sdk/EventSupport.java +++ b/src/main/java/dev/openfeature/sdk/EventSupport.java @@ -1,12 +1,12 @@ package dev.openfeature.sdk; -import java.util.ArrayList; -import java.util.List; +import java.util.Collection; import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; @@ -23,13 +23,10 @@ class EventSupport { // we use a v4 uuid as a "placeholder" for anonymous clients, since // ConcurrentHashMap doesn't support nulls - private static final String defaultClientUuid = UUID.randomUUID().toString(); + private static final String DEFAULT_CLIENT_UUID = UUID.randomUUID().toString(); private final Map handlerStores = new ConcurrentHashMap<>(); private final HandlerStore globalHandlerStore = new HandlerStore(); - private final ExecutorService taskExecutor = Executors.newCachedThreadPool(runnable -> { - final Thread thread = new Thread(runnable); - return thread; - }); + private final ExecutorService taskExecutor = Executors.newCachedThreadPool(); /** * Run all the event handlers associated with this domain. @@ -40,11 +37,10 @@ class EventSupport { * @param eventDetails the event details */ public void runClientHandlers(String domain, ProviderEvent event, EventDetails eventDetails) { - domain = Optional.ofNullable(domain).orElse(defaultClientUuid); + domain = Optional.ofNullable(domain).orElse(DEFAULT_CLIENT_UUID); // run handlers if they exist Optional.ofNullable(handlerStores.get(domain)) - .filter(store -> Optional.of(store).isPresent()) .map(store -> store.handlerMap.get(event)) .ifPresent(handlers -> handlers.forEach(handler -> runHandler(handler, eventDetails))); } @@ -69,7 +65,7 @@ public void runGlobalHandlers(ProviderEvent event, EventDetails eventDetails) { * @param handler the handler function to run */ public void addClientHandler(String domain, ProviderEvent event, Consumer handler) { - final String name = Optional.ofNullable(domain).orElse(defaultClientUuid); + final String name = Optional.ofNullable(domain).orElse(DEFAULT_CLIENT_UUID); // lazily create and cache a HandlerStore if it doesn't exist HandlerStore store = Optional.ofNullable(this.handlerStores.get(name)).orElseGet(() -> { @@ -89,7 +85,7 @@ public void addClientHandler(String domain, ProviderEvent event, Consumer handler) { - domain = Optional.ofNullable(domain).orElse(defaultClientUuid); + domain = Optional.ofNullable(domain).orElse(DEFAULT_CLIENT_UUID); this.handlerStores.get(domain).removeHandler(event, handler); } @@ -160,14 +156,14 @@ public void shutdown() { // instantiated when a handler is added to that client. static class HandlerStore { - private final Map>> handlerMap; + private final Map>> handlerMap; HandlerStore() { handlerMap = new ConcurrentHashMap<>(); - handlerMap.put(ProviderEvent.PROVIDER_READY, new ArrayList<>()); - 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<>()); + handlerMap.put(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, new ConcurrentLinkedQueue<>()); + handlerMap.put(ProviderEvent.PROVIDER_ERROR, new ConcurrentLinkedQueue<>()); + handlerMap.put(ProviderEvent.PROVIDER_STALE, new ConcurrentLinkedQueue<>()); } void addHandler(ProviderEvent event, Consumer handler) { diff --git a/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java b/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java index 32fa605c2..c954c8b19 100644 --- a/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java +++ b/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java @@ -150,7 +150,7 @@ void shouldPutTheProviderInStateErrorAfterEmittingErrorEvent() { api.setProviderAndWait(domain, provider); Client client = api.getClient(domain); assertThat(client.getProviderState()).isEqualTo(ProviderState.READY); - provider.emitProviderError(ProviderEventDetails.builder().build()); + provider.emitProviderError(ProviderEventDetails.builder().build()).await(); assertThat(client.getProviderState()).isEqualTo(ProviderState.ERROR); } @@ -165,7 +165,7 @@ void shouldPutTheProviderInStateStaleAfterEmittingStaleEvent() { api.setProviderAndWait(domain, provider); Client client = api.getClient(domain); assertThat(client.getProviderState()).isEqualTo(ProviderState.READY); - provider.emitProviderStale(ProviderEventDetails.builder().build()); + provider.emitProviderStale(ProviderEventDetails.builder().build()).await(); assertThat(client.getProviderState()).isEqualTo(ProviderState.STALE); } @@ -180,9 +180,9 @@ void shouldPutTheProviderInStateReadyAfterEmittingReadyEvent() { api.setProviderAndWait(domain, provider); Client client = api.getClient(domain); assertThat(client.getProviderState()).isEqualTo(ProviderState.READY); - provider.emitProviderStale(ProviderEventDetails.builder().build()); + provider.emitProviderStale(ProviderEventDetails.builder().build()).await(); assertThat(client.getProviderState()).isEqualTo(ProviderState.STALE); - provider.emitProviderReady(ProviderEventDetails.builder().build()); + provider.emitProviderReady(ProviderEventDetails.builder().build()).await(); assertThat(client.getProviderState()).isEqualTo(ProviderState.READY); } } diff --git a/src/test/java/dev/openfeature/sdk/EventProviderTest.java b/src/test/java/dev/openfeature/sdk/EventProviderTest.java index ebf8901cb..7a2bd8b84 100644 --- a/src/test/java/dev/openfeature/sdk/EventProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/EventProviderTest.java @@ -32,6 +32,7 @@ public static void resetDefaultProvider() { } @Test + @Timeout(2) @DisplayName("should run attached onEmit with emitters") void emitsEventsWhenAttached() { TriConsumer onEmit = mockOnEmit(); diff --git a/src/test/java/dev/openfeature/sdk/EventsTest.java b/src/test/java/dev/openfeature/sdk/EventsTest.java index 08c067f6e..b232f1177 100644 --- a/src/test/java/dev/openfeature/sdk/EventsTest.java +++ b/src/test/java/dev/openfeature/sdk/EventsTest.java @@ -604,7 +604,7 @@ void matchingStaleEventsMustRunImmediately() { TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); Client client = api.getClient(name); api.setProviderAndWait(name, provider); - provider.emitProviderStale(ProviderEventDetails.builder().build()); + provider.emitProviderStale(ProviderEventDetails.builder().build()).await(); assertThat(client.getProviderState()).isEqualTo(ProviderState.STALE); // should run even though handler was added after stale @@ -625,7 +625,7 @@ void matchingErrorEventsMustRunImmediately() { TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); Client client = api.getClient(name); api.setProviderAndWait(name, provider); - provider.emitProviderError(ProviderEventDetails.builder().build()); + provider.emitProviderError(ProviderEventDetails.builder().build()).await(); assertThat(client.getProviderState()).isEqualTo(ProviderState.ERROR); verify(handler, never()).accept(any()); diff --git a/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java b/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java index 4d2a8b287..970495940 100644 --- a/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java @@ -3,9 +3,14 @@ import static dev.openfeature.sdk.Structure.mapToStructure; import static dev.openfeature.sdk.testutils.TestFlagsUtils.buildFlags; import static org.awaitility.Awaitility.await; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.argThat; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import com.google.common.collect.ImmutableMap; import dev.openfeature.sdk.Client; @@ -19,6 +24,7 @@ import dev.openfeature.sdk.exceptions.TypeMismatchError; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import lombok.SneakyThrows; import org.junit.jupiter.api.BeforeEach; @@ -34,10 +40,11 @@ class InMemoryProviderTest { @SneakyThrows @BeforeEach void beforeEach() { + final var configChangedEventCounter = new AtomicInteger(); Map> flags = buildFlags(); provider = spy(new InMemoryProvider(flags)); api = OpenFeatureAPITestUtil.createAPI(); - api.onProviderConfigurationChanged(eventDetails -> {}); + api.onProviderConfigurationChanged(eventDetails -> configChangedEventCounter.incrementAndGet()); api.setProviderAndWait(provider); client = api.getClient(); provider.updateFlags(flags); @@ -48,6 +55,11 @@ void beforeEach() { .variant("off", false) .defaultVariant("on") .build()); + + // wait for the two config changed events to be fired, otherwise they could mess with our tests + while (configChangedEventCounter.get() < 2) { + Thread.sleep(1); + } } @Test From 43c06f31afca437b7db476d9e45d1478167831a9 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Mon, 2 Jun 2025 15:02:08 +0200 Subject: [PATCH 05/15] Flaky build due to a possible race condition #1449 Signed-off-by: christian.lutnik --- .../dev/openfeature/sdk/AwaitableTest.java | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 src/test/java/dev/openfeature/sdk/AwaitableTest.java diff --git a/src/test/java/dev/openfeature/sdk/AwaitableTest.java b/src/test/java/dev/openfeature/sdk/AwaitableTest.java new file mode 100644 index 000000000..c8fc6b6d5 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/AwaitableTest.java @@ -0,0 +1,75 @@ +package dev.openfeature.sdk; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +@Timeout(2) +class AwaitableTest { + @Test + void waitingForFinishedIsANoOp() { + var startTime = System.currentTimeMillis(); + Awaitable.FINISHED.await(); + var endTime = System.currentTimeMillis(); + assertTrue(endTime - startTime < 10); + } + + @Test + void waitingForNotFinishedWaitsEvenWhenInterrupted() throws InterruptedException { + var awaitable = new Awaitable(); + var mayProceed = new AtomicBoolean(false); + + var thread = new Thread(() -> { + awaitable.await(); + if (!mayProceed.get()) { + fail(); + } + }); + thread.start(); + + var startTime = System.currentTimeMillis(); + do { + thread.interrupt(); + } while (startTime + 1000 > System.currentTimeMillis()); + mayProceed.set(true); + awaitable.wakeup(); + thread.join(); + } + + @Test + void callingWakeUpWakesUpAllWaitingThreads() throws InterruptedException { + var awaitable = new Awaitable(); + var isRunning = new AtomicInteger(); + + Runnable runnable = () -> { + isRunning.incrementAndGet(); + var start = System.currentTimeMillis(); + awaitable.await(); + var end = System.currentTimeMillis(); + if (end - start > 10) { + fail(); + } + }; + + var numThreads = 2; + var threads = new Thread[numThreads]; + for (int i = 0; i < numThreads; i++) { + threads[i] = new Thread(runnable); + threads[i].start(); + } + + while (isRunning.get() < numThreads) { + Thread.sleep(1); + } + + awaitable.wakeup(); + + for (int i = 0; i < numThreads; i++) { + threads[i].join(); + } + } +} From 9e2cbd815da75c07e5849ce6dc602f91a5d5a888 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Mon, 2 Jun 2025 15:06:17 +0200 Subject: [PATCH 06/15] Flaky build due to a possible race condition #1449 Signed-off-by: christian.lutnik --- src/main/java/dev/openfeature/sdk/Awaitable.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/main/java/dev/openfeature/sdk/Awaitable.java b/src/main/java/dev/openfeature/sdk/Awaitable.java index f5ea45366..7176838c9 100644 --- a/src/main/java/dev/openfeature/sdk/Awaitable.java +++ b/src/main/java/dev/openfeature/sdk/Awaitable.java @@ -1,5 +1,8 @@ package dev.openfeature.sdk; +/** + * A class to help with synchronization + */ public class Awaitable { public static final Awaitable FINISHED = new Awaitable(true); @@ -11,6 +14,11 @@ private Awaitable(boolean isDone) { this.isDone = isDone; } + /** + * Lets the calling thread wait until some other thread calls {@link Awaitable#wakeup()}. If + * {@link Awaitable#wakeup()} has been called before the current thread invokes this method, it will return + * immediately + */ public void await() { if (isDone) { return; @@ -25,6 +33,9 @@ public void await() { } } + /** + * Wakes up all threads that have called {@link Awaitable#await()} and lets them proceed. + */ public synchronized void wakeup() { isDone = true; this.notifyAll(); From 694526e3e4ba3dcd3ed9abacd4698dcebabb1801 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Mon, 2 Jun 2025 15:15:20 +0200 Subject: [PATCH 07/15] Flaky build due to a possible race condition #1449 Signed-off-by: christian.lutnik --- src/main/java/dev/openfeature/sdk/Awaitable.java | 4 ++-- src/test/java/dev/openfeature/sdk/AwaitableTest.java | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/Awaitable.java b/src/main/java/dev/openfeature/sdk/Awaitable.java index 7176838c9..ab219aed0 100644 --- a/src/main/java/dev/openfeature/sdk/Awaitable.java +++ b/src/main/java/dev/openfeature/sdk/Awaitable.java @@ -1,7 +1,7 @@ package dev.openfeature.sdk; /** - * A class to help with synchronization + * A class to help with synchronization. */ public class Awaitable { public static final Awaitable FINISHED = new Awaitable(true); @@ -17,7 +17,7 @@ private Awaitable(boolean isDone) { /** * Lets the calling thread wait until some other thread calls {@link Awaitable#wakeup()}. If * {@link Awaitable#wakeup()} has been called before the current thread invokes this method, it will return - * immediately + * immediately. */ public void await() { if (isDone) { diff --git a/src/test/java/dev/openfeature/sdk/AwaitableTest.java b/src/test/java/dev/openfeature/sdk/AwaitableTest.java index c8fc6b6d5..80d932ce3 100644 --- a/src/test/java/dev/openfeature/sdk/AwaitableTest.java +++ b/src/test/java/dev/openfeature/sdk/AwaitableTest.java @@ -1,8 +1,10 @@ package dev.openfeature.sdk; +import static org.awaitility.Awaitility.await; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Test; @@ -62,9 +64,7 @@ void callingWakeUpWakesUpAllWaitingThreads() throws InterruptedException { threads[i].start(); } - while (isRunning.get() < numThreads) { - Thread.sleep(1); - } + await().atMost(1, TimeUnit.SECONDS).until(() -> isRunning.get() == numThreads); awaitable.wakeup(); From 95d2271e784f4d2927d46a7bb8224c2596e6b4e3 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Mon, 2 Jun 2025 15:18:11 +0200 Subject: [PATCH 08/15] Flaky build due to a possible race condition #1449 Signed-off-by: christian.lutnik --- src/main/java/dev/openfeature/sdk/Awaitable.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/dev/openfeature/sdk/Awaitable.java b/src/main/java/dev/openfeature/sdk/Awaitable.java index ab219aed0..7e86185a5 100644 --- a/src/main/java/dev/openfeature/sdk/Awaitable.java +++ b/src/main/java/dev/openfeature/sdk/Awaitable.java @@ -28,6 +28,7 @@ public void await() { try { this.wait(); } catch (InterruptedException ignored) { + // ignore } } } From 0738f9fb014b932c51a8e55eb116c8c0dd4fd3a2 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Mon, 2 Jun 2025 15:20:50 +0200 Subject: [PATCH 09/15] Flaky build due to a possible race condition #1449 Signed-off-by: christian.lutnik --- src/main/java/dev/openfeature/sdk/Awaitable.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/dev/openfeature/sdk/Awaitable.java b/src/main/java/dev/openfeature/sdk/Awaitable.java index 7e86185a5..fb548fd1b 100644 --- a/src/main/java/dev/openfeature/sdk/Awaitable.java +++ b/src/main/java/dev/openfeature/sdk/Awaitable.java @@ -28,7 +28,7 @@ public void await() { try { this.wait(); } catch (InterruptedException ignored) { - // ignore + Thread.currentThread().interrupt(); } } } From 3a1ce0949ada9cc69b75a22fada0526a9350965f Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Mon, 2 Jun 2025 17:05:12 +0200 Subject: [PATCH 10/15] Flaky build due to a possible race condition #1449 Signed-off-by: christian.lutnik --- src/main/java/dev/openfeature/sdk/Awaitable.java | 3 ++- src/main/java/dev/openfeature/sdk/EventProvider.java | 2 ++ src/test/java/dev/openfeature/sdk/AwaitableTest.java | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/Awaitable.java b/src/main/java/dev/openfeature/sdk/Awaitable.java index fb548fd1b..58874cea6 100644 --- a/src/main/java/dev/openfeature/sdk/Awaitable.java +++ b/src/main/java/dev/openfeature/sdk/Awaitable.java @@ -19,6 +19,7 @@ private Awaitable(boolean isDone) { * {@link Awaitable#wakeup()} has been called before the current thread invokes this method, it will return * immediately. */ + @SuppressWarnings("java:S2142") public void await() { if (isDone) { return; @@ -28,7 +29,7 @@ public void await() { try { this.wait(); } catch (InterruptedException ignored) { - Thread.currentThread().interrupt(); + // ignored, do not propagate the interrupted state } } } diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index dc28ee5ae..0d7e897c2 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -86,6 +86,8 @@ public Awaitable emit(final ProviderEvent event, final ProviderEventDetails deta final var awaitable = new Awaitable(); + // These calls need to be executed on a different thread to prevent deadlocks when the provider initialization + // relies on a ready event to be emitted emitterExecutor.submit(() -> { try (var ignored = OpenFeatureAPI.lock.readLockAutoCloseable()) { if (localEventProviderListener != null) { diff --git a/src/test/java/dev/openfeature/sdk/AwaitableTest.java b/src/test/java/dev/openfeature/sdk/AwaitableTest.java index 80d932ce3..17a896fc7 100644 --- a/src/test/java/dev/openfeature/sdk/AwaitableTest.java +++ b/src/test/java/dev/openfeature/sdk/AwaitableTest.java @@ -10,7 +10,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; -@Timeout(2) +@Timeout(value=5, threadMode = Timeout.ThreadMode.SEPARATE_THREAD) class AwaitableTest { @Test void waitingForFinishedIsANoOp() { From 5ab1f68440f44f8c5f97f34d79d679ec6b611d78 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Mon, 2 Jun 2025 17:06:03 +0200 Subject: [PATCH 11/15] Flaky build due to a possible race condition #1449 Signed-off-by: christian.lutnik --- src/test/java/dev/openfeature/sdk/EventProviderTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/dev/openfeature/sdk/EventProviderTest.java b/src/test/java/dev/openfeature/sdk/EventProviderTest.java index 7a2bd8b84..d04fa88d1 100644 --- a/src/test/java/dev/openfeature/sdk/EventProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/EventProviderTest.java @@ -32,7 +32,7 @@ public static void resetDefaultProvider() { } @Test - @Timeout(2) + @Timeout(value = 2, threadMode = Timeout.ThreadMode.SEPARATE_THREAD) @DisplayName("should run attached onEmit with emitters") void emitsEventsWhenAttached() { TriConsumer onEmit = mockOnEmit(); From 1feacddcbe5eceb01981d1d9557ad50b3319f763 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Mon, 2 Jun 2025 17:28:44 +0200 Subject: [PATCH 12/15] Flaky build due to a possible race condition #1449 Signed-off-by: christian.lutnik --- .../java/dev/openfeature/sdk/Awaitable.java | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/Awaitable.java b/src/main/java/dev/openfeature/sdk/Awaitable.java index 58874cea6..689ad9bcd 100644 --- a/src/main/java/dev/openfeature/sdk/Awaitable.java +++ b/src/main/java/dev/openfeature/sdk/Awaitable.java @@ -20,17 +20,12 @@ private Awaitable(boolean isDone) { * immediately. */ @SuppressWarnings("java:S2142") - public void await() { - if (isDone) { - return; - } - synchronized (this) { - while (!isDone) { - try { - this.wait(); - } catch (InterruptedException ignored) { - // ignored, do not propagate the interrupted state - } + public synchronized void await() { + while (!isDone) { + try { + this.wait(); + } catch (InterruptedException ignored) { + // ignored, do not propagate the interrupted state } } } From 6a4fa3df4038655913ae17876393f68ef96a1b2d Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Tue, 3 Jun 2025 08:24:56 +0200 Subject: [PATCH 13/15] Flaky build due to a possible race condition #1449 Signed-off-by: christian.lutnik --- src/test/java/dev/openfeature/sdk/AwaitableTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/dev/openfeature/sdk/AwaitableTest.java b/src/test/java/dev/openfeature/sdk/AwaitableTest.java index 17a896fc7..70ef7902c 100644 --- a/src/test/java/dev/openfeature/sdk/AwaitableTest.java +++ b/src/test/java/dev/openfeature/sdk/AwaitableTest.java @@ -10,7 +10,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; -@Timeout(value=5, threadMode = Timeout.ThreadMode.SEPARATE_THREAD) +@Timeout(value = 5, threadMode = Timeout.ThreadMode.SEPARATE_THREAD) class AwaitableTest { @Test void waitingForFinishedIsANoOp() { From 596fe227b1b9d8b8c7aad4497495c5afdb52c863 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Tue, 10 Jun 2025 11:18:09 -0400 Subject: [PATCH 14/15] Update src/main/java/dev/openfeature/sdk/Awaitable.java Signed-off-by: Todd Baert --- src/main/java/dev/openfeature/sdk/Awaitable.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/dev/openfeature/sdk/Awaitable.java b/src/main/java/dev/openfeature/sdk/Awaitable.java index 689ad9bcd..ce48de46b 100644 --- a/src/main/java/dev/openfeature/sdk/Awaitable.java +++ b/src/main/java/dev/openfeature/sdk/Awaitable.java @@ -1,7 +1,7 @@ package dev.openfeature.sdk; /** - * A class to help with synchronization. + * A class to help with synchronization by allowing the optional awaiting of the associated action. */ public class Awaitable { public static final Awaitable FINISHED = new Awaitable(true); From 0228a7038ba877cb8b78d6150c8000a23d02bec9 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Tue, 10 Jun 2025 11:19:37 -0400 Subject: [PATCH 15/15] Update src/main/java/dev/openfeature/sdk/Awaitable.java Signed-off-by: Todd Baert --- src/main/java/dev/openfeature/sdk/Awaitable.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/dev/openfeature/sdk/Awaitable.java b/src/main/java/dev/openfeature/sdk/Awaitable.java index ce48de46b..7d5f477dc 100644 --- a/src/main/java/dev/openfeature/sdk/Awaitable.java +++ b/src/main/java/dev/openfeature/sdk/Awaitable.java @@ -4,6 +4,10 @@ * A class to help with synchronization by allowing the optional awaiting of the associated action. */ public class Awaitable { + + /** + * An already-completed Awaitable. Awaiting this will return immediately. + */ public static final Awaitable FINISHED = new Awaitable(true); private boolean isDone = false;