Skip to content

Commit e8f348a

Browse files
committed
fixup: simplify EventProvider.attach to single CAS
per chrfwow's review suggestion. The previous CAS loop existed to support a no-op-same-onEmit branch which is unreachable in practice: attach() is package-private (not part of the public API), and the sole production caller (OpenFeatureAPI.attachEventProvider) always passes a fresh method-reference. Drops the now-orphaned doesNotThrowWhenOnEmitSame test. Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
1 parent 2b4135f commit e8f348a

2 files changed

Lines changed: 3 additions & 21 deletions

File tree

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

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ private static final class Attachment {
4949

5050
/**
5151
* "Attach" this EventProvider to an SDK, which allows events to propagate from this provider.
52-
* No-op if the same onEmit is already attached.
5352
*
5453
* @param onEmit the function to run when a provider emits events.
5554
* @param lock the API instance's read/write lock for thread safety.
@@ -59,16 +58,9 @@ void attach(
5958
TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit,
6059
AutoCloseableReentrantReadWriteLock lock) {
6160
Attachment newAttachment = new Attachment(onEmit, lock);
62-
while (true) {
63-
Attachment existing = this.attachment.get();
64-
if (existing != null && existing.onEmit != onEmit) {
65-
// if we are trying to attach this provider to a different onEmit, something has gone wrong
66-
throw new IllegalStateException(
67-
"Provider " + this.getMetadata().getName() + " is already attached.");
68-
}
69-
if (this.attachment.compareAndSet(existing, newAttachment)) {
70-
return;
71-
}
61+
if (!this.attachment.compareAndSet(null, newAttachment)) {
62+
throw new IllegalStateException(
63+
"Provider " + this.getMetadata().getName() + " is already attached.");
7264
}
7365
}
7466

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -82,16 +82,6 @@ void throwsWhenOnEmitDifferent() {
8282
assertThrows(IllegalStateException.class, () -> eventProvider.attach(onEmit2, lock));
8383
}
8484

85-
@Test
86-
@DisplayName("should not throw if second same onEmit attached")
87-
void doesNotThrowWhenOnEmitSame() {
88-
TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit1 = mockOnEmit();
89-
TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit2 = onEmit1;
90-
eventProvider.attach(onEmit1, new AutoCloseableReentrantReadWriteLock());
91-
eventProvider.attach(
92-
onEmit2, new AutoCloseableReentrantReadWriteLock()); // should not throw, same instance. noop
93-
}
94-
9585
@Test
9686
@Timeout(value = 2, threadMode = Timeout.ThreadMode.SEPARATE_THREAD)
9787
@DisplayName("emit should acquire read lock when attached")

0 commit comments

Comments
 (0)