Skip to content

Commit 373eac5

Browse files
fix: Do not emit a redundant ready event during initialization (#63)
Stops the provider from emitting a second `PROVIDER_READY` event during initialization, and makes `LifeCycleTest.itCanHandleClientThatIsNotInitializedImmediately` wait for the event instead of racing it. - The provider emitted ready from its data source status listener while `initialize` was still running, and the OpenFeature SDK emits its own ready event when initialization succeeds — consumers got two. - The test only passed because it asserted the counter before the second event was dispatched; adding a wait made it fail `expected: <1> but was: <2>` 25/25 times, which is how the double emit surfaced. - Ready events for later recoveries (stale/error back to valid) are unaffected — only the emit that happens while `initialize` is in flight is suppressed. - Under CPU load, `LifeCycleTest` now passes 25/25; before this branch it failed 9/25 with `expected: <1> but was: <0>`. <details> <summary>Implementation details</summary> **Requirements** - [x] I have added test coverage for new or changed functionality - [x] I have followed the repository's [pull request submission guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests) - [x] I have validated my changes against all supported platform versions **Mechanism** `initialize` sets an `initializing` flag for the duration of the call. In the `VALID` branch of `handleDataSourceStatus` the flag is read *before* completing the initialization future, so the initialize thread cannot clear it between the read and the emit: ```java if (emit) { boolean duringInitialization = initializing; completer.complete(true); if (!duringInitialization) { emitProviderReady(...); } } ``` `setProviderAndWait` also returns before API-level handlers have run, since the OpenFeature SDK dispatches events on its own executor; the test now waits on a `CompletableFuture` (1s timeout) and then asserts the count after a short grace period so a duplicate emit is still caught. **Alternatives rejected** Dropping the exact-count assertion from the test — it hides exactly the duplicate-emit bug this change fixes. **Testing** `./gradlew test --tests '*LifeCycleTest*' --rerun-tasks` repeated 25x with six busy loops pinning the CPUs, and the full suite once: all green. On `main` the same loop failed 9/25 (and 5/20 in an earlier round). CI on this branch was also re-triggered 11 times with empty commits, all green, before the provider change. **Related** Found while investigating #32 (debug output for a flaky `itEmitsReadyEvents`); that flake did not reproduce in ~75 runs and its debug prints are superseded by #59, so #32 looks closable. </details> Link to Devin session: https://app.devin.ai/sessions/3c9b094de4f84a0f82ba266098e823e9 Open in Devin Desktop: https://app.devin.ai/desktop/session/3c9b094de4f84a0f82ba266098e823e9?variant=devin Requested by: @kinyoklion <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > Fixes **double `PROVIDER_READY` events** when the LaunchDarkly data source becomes valid while `initialize` is still running. The OpenFeature SDK already emits ready on successful initialization; the provider was also calling `emitProviderReady` from the data-source status listener, so listeners could see two events. > > An **`initializing` flag** wraps the wait in `initialize` (cleared in `finally`). On transition to `VALID`, the provider still completes initialization and updates state, but **skips `emitProviderReady` while `initializing` is true**. Recoveries after init (e.g. stale/error back to valid) still emit ready as before. > > **Tests:** `itCanHandleClientThatIsNotInitializedImmediately` now waits for the ready event and rechecks the count after a short delay to catch races. A new case uses a controllable data source to assert that **failed init followed by `VALID` still produces exactly one ready event**. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit f8a6052. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 parent 1f35bc8 commit 373eac5

2 files changed

Lines changed: 102 additions & 6 deletions

File tree

src/main/java/com/launchdarkly/openfeature/serverprovider/Provider.java

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ public String getName() {
5151

5252
private final Object stateLock = new Object();
5353

54+
private boolean initializing = false;
55+
5456
/**
5557
* Create a provider with the specified SDK and default configuration.
5658
* <p>
@@ -152,6 +154,7 @@ public void initialize(EvaluationContext evaluationContext) throws Exception {
152154
setState(ProviderState.READY);
153155
}
154156

157+
setInitializing(true);
155158
var completer = new CompletableFuture<Boolean>();
156159

157160
client.getFlagTracker().addFlagChangeListener(detail -> {
@@ -164,11 +167,17 @@ public void initialize(EvaluationContext evaluationContext) throws Exception {
164167
});
165168

166169
if (getState() == ProviderState.READY) {
170+
setInitializing(false);
167171
return;
168172
}
169173

170-
handleDataSourceStatus(client.getDataSourceStatusProvider().getStatus(), completer);
171-
var successfullyInitialized = completer.get();
174+
boolean successfullyInitialized;
175+
try {
176+
handleDataSourceStatus(client.getDataSourceStatusProvider().getStatus(), completer);
177+
successfullyInitialized = completer.get();
178+
} finally {
179+
setInitializing(false);
180+
}
172181

173182
if(!successfullyInitialized) {
174183
throw new RuntimeException("Failed to initialize LaunchDarkly client.");
@@ -189,19 +198,24 @@ private void handleDataSourceStatus(DataSourceStatusProvider.Status res, Complet
189198
}
190199
break;
191200
case VALID: {
201+
boolean becameReady = false;
192202
boolean emit = false;
193203
synchronized (stateLock) {
194204
// If we are ready, then we don't want to emit it again. Other conditions we may be updating the
195205
// reason we are stale or interrupted, so we want to emit an event each time.
196206
if (state != ProviderState.READY) {
197-
emit = true;
198-
setState(ProviderState.READY);
207+
becameReady = true;
208+
// The OpenFeature SDK emits its own ready event when initialization succeeds.
209+
emit = !initializing;
210+
state = ProviderState.READY;
199211
}
200212
}
201213

202-
if (emit) {
214+
if (becameReady) {
203215
completer.complete(true);
204-
emitProviderReady(ProviderEventDetails.builder().build());
216+
if (emit) {
217+
emitProviderReady(ProviderEventDetails.builder().build());
218+
}
205219
}
206220
}
207221
break;
@@ -218,6 +232,12 @@ private void handleDataSourceStatus(DataSourceStatusProvider.Status res, Complet
218232
}
219233
}
220234

235+
private void setInitializing(boolean initializing) {
236+
synchronized (stateLock) {
237+
this.initializing = initializing;
238+
}
239+
}
240+
221241
private void setState(ProviderState state) {
222242
synchronized (stateLock) {
223243
this.state = state;

src/test/java/com/launchdarkly/openfeature/serverprovider/LifeCycleTest.java

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,29 @@ public void close() throws IOException {
104104
}
105105
}
106106

107+
class ControllableDataSource implements DataSource {
108+
public Future<Void> start() {
109+
return new CompletableFuture<>();
110+
}
111+
112+
public boolean isInitialized() {
113+
return false;
114+
}
115+
116+
public void close() throws IOException {
117+
}
118+
}
119+
120+
class ControllableDataSourceFactory implements ComponentConfigurer<DataSource> {
121+
final CompletableFuture<DataSourceUpdateSink> sink = new CompletableFuture<>();
122+
123+
@Override
124+
public DataSource build(ClientContext clientContext) {
125+
sink.complete(clientContext.getDataSourceUpdateSink());
126+
return new ControllableDataSource();
127+
}
128+
}
129+
107130
class DelayedDataSourceFactory implements ComponentConfigurer<DataSource> {
108131
private Duration startDelay;
109132
private boolean willError;
@@ -219,14 +242,19 @@ public void itCanHandleClientThatIsNotInitializedImmediately() throws Exception
219242
assertEquals(ProviderState.NOT_READY, provider.getState());
220243

221244
var readyCount = new AtomicInteger();
245+
CompletableFuture<Boolean> gotReadyEvent = new CompletableFuture<>();
222246

223247
OpenFeatureAPI.getInstance().on(ProviderEvent.PROVIDER_READY, (detail) -> {
224248
readyCount.getAndIncrement();
249+
gotReadyEvent.complete(true);
225250
});
226251

227252
OpenFeatureAPI.getInstance().setProviderAndWait(provider);
228253

229254
assertEquals(ProviderState.READY, provider.getState());
255+
assertTrue(gotReadyEvent.get(1000, TimeUnit.MILLISECONDS));
256+
257+
Thread.sleep(100);
230258
assertEquals(1, readyCount.get());
231259
}
232260

@@ -260,6 +288,54 @@ public void itCanHandleClientThatIsNotInitializedImmediatelyAndErrors() throws E
260288
assertTrue(gotErrorEvent.get(1000, TimeUnit.MILLISECONDS));
261289
}
262290

291+
@Test
292+
public void itEmitsReadyWhenTheDataSourceRecoversFromAFailedInitialization() throws Exception {
293+
var dataSourceFactory = new ControllableDataSourceFactory();
294+
var config = new LDConfig.Builder()
295+
.startWait(Duration.ZERO)
296+
.dataSource(dataSourceFactory)
297+
.events(Components.noEvents())
298+
.build();
299+
var provider = new Provider("fake-key", config);
300+
var sink = dataSourceFactory.sink.get(1000, TimeUnit.MILLISECONDS);
301+
302+
var readyCount = new AtomicInteger();
303+
CompletableFuture<Boolean> gotReadyEvent = new CompletableFuture<>();
304+
CompletableFuture<Boolean> gotErrorEvent = new CompletableFuture<>();
305+
306+
OpenFeatureAPI.getInstance().on(ProviderEvent.PROVIDER_READY, (detail) -> {
307+
readyCount.getAndIncrement();
308+
gotReadyEvent.complete(true);
309+
});
310+
311+
OpenFeatureAPI.getInstance().on(ProviderEvent.PROVIDER_ERROR, (detail) -> {
312+
gotErrorEvent.complete(true);
313+
});
314+
315+
sink.updateStatus(DataSourceStatusProvider.State.OFF, new DataSourceStatusProvider.ErrorInfo(
316+
DataSourceStatusProvider.ErrorKind.NETWORK_ERROR,
317+
404,
318+
"bad",
319+
LocalDateTime.now().toInstant(ZoneOffset.UTC)));
320+
321+
GeneralError initializationError = null;
322+
try {
323+
OpenFeatureAPI.getInstance().setProviderAndWait(provider);
324+
} catch (GeneralError e) {
325+
initializationError = e;
326+
}
327+
328+
assertNotNull(initializationError);
329+
assertTrue(gotErrorEvent.get(1000, TimeUnit.MILLISECONDS));
330+
331+
sink.updateStatus(DataSourceStatusProvider.State.VALID, null);
332+
333+
assertTrue(gotReadyEvent.get(1000, TimeUnit.MILLISECONDS));
334+
335+
Thread.sleep(100);
336+
assertEquals(1, readyCount.get());
337+
}
338+
263339
@Test
264340
public void itIncludesTheDataSourceErrorInErrorEvents() throws Exception {
265341
var config = new LDConfig.Builder()

0 commit comments

Comments
 (0)