Skip to content

Commit 4b0d22d

Browse files
fix(java): don't recreate WASM instance on thread interruption (#381)
1 parent a427545 commit 4b0d22d

4 files changed

Lines changed: 101 additions & 21 deletions

File tree

openfeature-provider/java/src/main/java/com/spotify/confidence/sdk/OpenFeatureLocalResolveProvider.java

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -203,8 +203,12 @@ public void initialize(EvaluationContext evaluationContext) {
203203

204204
assignLogExecutor.scheduleAtFixedRate(
205205
() -> {
206-
if (initialized) {
207-
resolver.flushAssignLogs();
206+
try {
207+
if (initialized) {
208+
resolver.flushAssignLogs();
209+
}
210+
} catch (RuntimeException e) {
211+
log.error("Failed to flush assign logs", e);
208212
}
209213
},
210214
ASSIGN_LOG_FLUSH_INTERVAL.toMillis(),
@@ -225,24 +229,28 @@ private void scheduleStateRefresh(
225229

226230
flagsFetcherExecutor.schedule(
227231
() -> {
228-
stateProvider.reload();
229-
resolverStateProtobuf.set(stateProvider.provide());
230-
accountIdRef.set(stateProvider.accountId());
231-
232-
if (!accountIdRef.get().isEmpty()) {
233-
if (!initialized) {
234-
resolver.setResolverState(resolverStateProtobuf.get(), accountIdRef.get(), SDK);
235-
initialized = true;
236-
this.state.set(ProviderState.READY);
237-
log.info("Provider recovered and is now READY");
238-
} else {
239-
// State refresh + full log flush
240-
resolver.setResolverState(resolverStateProtobuf.get(), accountIdRef.get(), SDK);
241-
resolver.flushAllLogs();
232+
try {
233+
stateProvider.reload();
234+
resolverStateProtobuf.set(stateProvider.provide());
235+
accountIdRef.set(stateProvider.accountId());
236+
237+
if (!accountIdRef.get().isEmpty()) {
238+
if (!initialized) {
239+
resolver.setResolverState(resolverStateProtobuf.get(), accountIdRef.get(), SDK);
240+
initialized = true;
241+
this.state.set(ProviderState.READY);
242+
log.info("Provider recovered and is now READY");
243+
} else {
244+
// State refresh + full log flush
245+
resolver.setResolverState(resolverStateProtobuf.get(), accountIdRef.get(), SDK);
246+
resolver.flushAllLogs();
247+
}
242248
}
249+
} catch (RuntimeException e) {
250+
log.error("State refresh failed", e);
251+
} finally {
252+
scheduleStateRefresh(resolverStateProtobuf, accountIdRef, pollIntervalSeconds);
243253
}
244-
245-
scheduleStateRefresh(resolverStateProtobuf, accountIdRef, pollIntervalSeconds);
246254
},
247255
delaySeconds,
248256
TimeUnit.SECONDS);

openfeature-provider/java/src/main/java/com/spotify/confidence/sdk/PooledResolver.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,14 @@ public void flushAssignLogs() {
8484

8585
@Override
8686
public void close() {
87-
maintenance(LocalResolver::close);
87+
maintenance(
88+
lr -> {
89+
try {
90+
lr.close();
91+
} catch (RuntimeException e) {
92+
logger.error("Failed to close resolver", e);
93+
}
94+
});
8895
}
8996

9097
@Override
@@ -131,8 +138,6 @@ private void maintenance(Consumer<LocalResolver> fn) {
131138
slot.rwLock.writeLock().lock();
132139
try {
133140
fn.accept(slot.resolver);
134-
} catch (RuntimeException e) {
135-
logger.error("Maintenance operation failed on slot {}", i, e);
136141
} finally {
137142
slot.rwLock.writeLock().unlock();
138143
}

openfeature-provider/java/src/main/java/com/spotify/confidence/sdk/RecoveringResolver.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.spotify.confidence.sdk;
22

3+
import com.dylibso.chicory.runtime.ChicoryInterruptedException;
34
import com.dylibso.chicory.wasm.ChicoryException;
45
import com.spotify.confidence.sdk.flags.resolver.v1.ApplyFlagsRequest;
56
import com.spotify.confidence.sdk.flags.resolver.v1.RegisterResolveRequest;
@@ -64,6 +65,10 @@ private void startRecreate() {
6465
}
6566

6667
private void handleFailure(String opName, ChicoryException e) {
68+
if (e instanceof ChicoryInterruptedException) {
69+
logger.debug("Resolver interrupted during {}, not recreating", opName);
70+
throw e;
71+
}
6772
if (broken.compareAndSet(false, true)) {
6873
logger.warn(
6974
"Resolver failed during {} ({}), starting background recreation",
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package com.spotify.confidence.sdk;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
5+
import static org.mockito.ArgumentMatchers.any;
6+
import static org.mockito.Mockito.*;
7+
8+
import com.dylibso.chicory.runtime.ChicoryInterruptedException;
9+
import com.dylibso.chicory.wasm.ChicoryException;
10+
import com.spotify.confidence.sdk.flags.resolver.v1.Sdk;
11+
import java.util.concurrent.atomic.AtomicInteger;
12+
import org.junit.jupiter.api.Test;
13+
14+
class RecoveringResolverTest {
15+
16+
private static final byte[] STATE = new byte[] {1, 2, 3};
17+
private static final String ACCOUNT_ID = "account";
18+
private static final Sdk SDK = Sdk.getDefaultInstance();
19+
20+
@Test
21+
void chicoryExceptionTriggersRecreation() throws Exception {
22+
final LocalResolver failing = mock(LocalResolver.class);
23+
doThrow(new ChicoryException("wasm trap")).when(failing).setResolverState(any(), any(), any());
24+
25+
final LocalResolver replacement = mock(LocalResolver.class);
26+
final AtomicInteger factoryCalls = new AtomicInteger();
27+
final RecoveringResolver recovering =
28+
new RecoveringResolver(
29+
() -> {
30+
int call = factoryCalls.incrementAndGet();
31+
return call == 1 ? failing : replacement;
32+
});
33+
34+
assertThatThrownBy(() -> recovering.setResolverState(STATE, ACCOUNT_ID, SDK))
35+
.isInstanceOf(ChicoryException.class);
36+
37+
Thread.sleep(200);
38+
assertThat(factoryCalls.get()).isEqualTo(2);
39+
}
40+
41+
@Test
42+
void interruptedExceptionDoesNotTriggerRecreation() throws Exception {
43+
final LocalResolver failing = mock(LocalResolver.class);
44+
doThrow(new ChicoryInterruptedException("Thread interrupted"))
45+
.when(failing)
46+
.setResolverState(any(), any(), any());
47+
48+
final AtomicInteger factoryCalls = new AtomicInteger();
49+
final RecoveringResolver recovering =
50+
new RecoveringResolver(
51+
() -> {
52+
factoryCalls.incrementAndGet();
53+
return failing;
54+
});
55+
56+
assertThatThrownBy(() -> recovering.setResolverState(STATE, ACCOUNT_ID, SDK))
57+
.isInstanceOf(ChicoryInterruptedException.class);
58+
59+
Thread.sleep(200);
60+
assertThat(factoryCalls.get()).isEqualTo(1);
61+
}
62+
}

0 commit comments

Comments
 (0)