Skip to content

Commit c06e500

Browse files
committed
refactor: bundle EventProvider onEmit and lock into atomic attachment
Signed-off-by: marcozabel <marco.zabel@dynatrace.com>
1 parent 5f14bb9 commit c06e500

2 files changed

Lines changed: 29 additions & 18 deletions

File tree

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

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,21 @@ void setEventProviderListener(EventProviderListener eventProviderListener) {
3030
this.eventProviderListener = eventProviderListener;
3131
}
3232

33-
private TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit = null;
34-
private AutoCloseableReentrantReadWriteLock lock = null;
33+
// Bundles onEmit and lock into a single volatile reference so they are always read atomically:
34+
// a non-null attachment guarantees a non-null lock.
35+
private static final class Attachment {
36+
final TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit;
37+
final AutoCloseableReentrantReadWriteLock lock;
38+
39+
Attachment(
40+
TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit,
41+
AutoCloseableReentrantReadWriteLock lock) {
42+
this.onEmit = onEmit;
43+
this.lock = lock;
44+
}
45+
}
46+
47+
private volatile Attachment attachment = null;
3548

3649
/**
3750
* "Attach" this EventProvider to an SDK, which allows events to propagate from this provider.
@@ -44,21 +57,19 @@ void setEventProviderListener(EventProviderListener eventProviderListener) {
4457
void attach(
4558
TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit,
4659
AutoCloseableReentrantReadWriteLock lock) {
47-
if (this.onEmit != null && this.onEmit != onEmit) {
60+
Attachment existing = this.attachment;
61+
if (existing != null && existing.onEmit != onEmit) {
4862
// if we are trying to attach this provider to a different onEmit, something has gone wrong
4963
throw new IllegalStateException("Provider " + this.getMetadata().getName() + " is already attached.");
50-
} else {
51-
this.onEmit = onEmit;
52-
this.lock = lock;
5364
}
65+
this.attachment = new Attachment(onEmit, lock);
5466
}
5567

5668
/**
5769
* "Detach" this EventProvider from an SDK, stopping propagation of all events.
5870
*/
5971
void detach() {
60-
this.onEmit = null;
61-
this.lock = null;
72+
this.attachment = null;
6273
}
6374

6475
/**
@@ -87,10 +98,9 @@ public void shutdown() {
8798
*/
8899
public Awaitable emit(final ProviderEvent event, final ProviderEventDetails details) {
89100
final var localEventProviderListener = this.eventProviderListener;
90-
final var localOnEmit = this.onEmit;
91-
final var localLock = this.lock;
101+
final var localAttachment = this.attachment;
92102

93-
if (localEventProviderListener == null && localOnEmit == null) {
103+
if (localEventProviderListener == null && localAttachment == null) {
94104
return Awaitable.FINISHED;
95105
}
96106

@@ -99,12 +109,14 @@ public Awaitable emit(final ProviderEvent event, final ProviderEventDetails deta
99109
// These calls need to be executed on a different thread to prevent deadlocks when the provider initialization
100110
// relies on a ready event to be emitted
101111
emitterExecutor.submit(() -> {
102-
try (var ignored = localLock != null ? localLock.readLockAutoCloseable() : null) {
112+
// Lock is only needed when attached to an API instance. A non-null attachment always
113+
// carries a non-null lock, so no null check on the lock itself is required.
114+
try (var ignored = localAttachment != null ? localAttachment.lock.readLockAutoCloseable() : null) {
103115
if (localEventProviderListener != null) {
104116
localEventProviderListener.onEmit(event, details);
105117
}
106-
if (localOnEmit != null) {
107-
localOnEmit.accept(this, event, details);
118+
if (localAttachment != null) {
119+
localAttachment.onEmit.accept(this, event, details);
108120
}
109121
} finally {
110122
awaitable.wakeup();

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,9 @@ void doesNotEmitsEventsWhenNotAttached() {
7474
void throwsWhenOnEmitDifferent() {
7575
TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit1 = mockOnEmit();
7676
TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit2 = mockOnEmit();
77-
eventProvider.attach(onEmit1, new AutoCloseableReentrantReadWriteLock());
78-
assertThrows(
79-
IllegalStateException.class,
80-
() -> eventProvider.attach(onEmit2, new AutoCloseableReentrantReadWriteLock()));
77+
AutoCloseableReentrantReadWriteLock lock = new AutoCloseableReentrantReadWriteLock();
78+
eventProvider.attach(onEmit1, lock);
79+
assertThrows(IllegalStateException.class, () -> eventProvider.attach(onEmit2, lock));
8180
}
8281

8382
@Test

0 commit comments

Comments
 (0)