JAMES-2659 add a test to demonstrate that synchronization on listeners is not well done
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/d0866371 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/d0866371 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/d0866371 Branch: refs/heads/master Commit: d0866371f9229e302eccc5058e734be2065edc28 Parents: 9797d16 Author: Matthieu Baechler <matth...@apache.org> Authored: Thu Feb 7 15:52:40 2019 +0100 Committer: Matthieu Baechler <matth...@apache.org> Committed: Thu Feb 7 17:10:39 2019 +0100 ---------------------------------------------------------------------- .../mailbox/events/MailboxListenerRegistry.java | 25 ++++++++++------- .../events/MailboxListenerRegistryTest.java | 28 ++++++++++++++++++++ 2 files changed, 44 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/d0866371/mailbox/event/event-rabbitmq/src/main/java/org/apache/james/mailbox/events/MailboxListenerRegistry.java ---------------------------------------------------------------------- diff --git a/mailbox/event/event-rabbitmq/src/main/java/org/apache/james/mailbox/events/MailboxListenerRegistry.java b/mailbox/event/event-rabbitmq/src/main/java/org/apache/james/mailbox/events/MailboxListenerRegistry.java index 63caf00..4b440db 100644 --- a/mailbox/event/event-rabbitmq/src/main/java/org/apache/james/mailbox/events/MailboxListenerRegistry.java +++ b/mailbox/event/event-rabbitmq/src/main/java/org/apache/james/mailbox/events/MailboxListenerRegistry.java @@ -20,6 +20,7 @@ package org.apache.james.mailbox.events; import com.google.common.collect.HashMultimap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Multimap; import com.google.common.collect.Multimaps; @@ -32,21 +33,27 @@ class MailboxListenerRegistry { this.listeners = Multimaps.synchronizedMultimap(HashMultimap.create()); } - synchronized void addListener(RegistrationKey registrationKey, MailboxListener listener, Runnable runIfEmpty) { - if (listeners.get(registrationKey).isEmpty()) { - runIfEmpty.run(); + void addListener(RegistrationKey registrationKey, MailboxListener listener, Runnable runIfEmpty) { + synchronized (listeners) { + if (listeners.get(registrationKey).isEmpty()) { + runIfEmpty.run(); + } + listeners.put(registrationKey, listener); } - listeners.put(registrationKey, listener); } - synchronized void removeListener(RegistrationKey registrationKey, MailboxListener listener, Runnable runIfEmpty) { - boolean wasRemoved = listeners.remove(registrationKey, listener); - if (wasRemoved && listeners.get(registrationKey).isEmpty()) { - runIfEmpty.run(); + void removeListener(RegistrationKey registrationKey, MailboxListener listener, Runnable runIfEmpty) { + synchronized (listeners) { + boolean wasRemoved = listeners.remove(registrationKey, listener); + if (wasRemoved && listeners.get(registrationKey).isEmpty()) { + runIfEmpty.run(); + } } } Flux<MailboxListener> getLocalMailboxListeners(RegistrationKey registrationKey) { - return Flux.fromIterable(listeners.get(registrationKey)); + synchronized (listeners) { + return Flux.fromIterable(ImmutableSet.copyOf(listeners.get(registrationKey))); + } } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/james-project/blob/d0866371/mailbox/event/event-rabbitmq/src/test/java/org/apache/james/mailbox/events/MailboxListenerRegistryTest.java ---------------------------------------------------------------------- diff --git a/mailbox/event/event-rabbitmq/src/test/java/org/apache/james/mailbox/events/MailboxListenerRegistryTest.java b/mailbox/event/event-rabbitmq/src/test/java/org/apache/james/mailbox/events/MailboxListenerRegistryTest.java index a63dd21..6320274 100644 --- a/mailbox/event/event-rabbitmq/src/test/java/org/apache/james/mailbox/events/MailboxListenerRegistryTest.java +++ b/mailbox/event/event-rabbitmq/src/test/java/org/apache/james/mailbox/events/MailboxListenerRegistryTest.java @@ -22,6 +22,7 @@ package org.apache.james.mailbox.events; import static org.assertj.core.api.Assertions.assertThat; import java.time.Duration; +import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; @@ -31,6 +32,9 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import reactor.core.publisher.Mono; +import reactor.core.scheduler.Schedulers; + class MailboxListenerRegistryTest { private static final MailboxIdRegistrationKey KEY_1 = new MailboxIdRegistrationKey(TestId.of(42)); private static final Runnable NOOP = () -> { @@ -235,5 +239,29 @@ class MailboxListenerRegistryTest { assertThat(runIfEmptyCount.get()).isEqualTo(1); } + + @Test + void iterationShouldPerformOnASnapshotOfListenersSet() throws Exception { + MailboxListener listener1 = event -> {}; + MailboxListener listener2 = event -> {}; + MailboxListener listener3 = event -> {}; + MailboxListener listener4 = event -> {}; + MailboxListener listener5 = event -> {}; + + testee.addListener(KEY_1, listener1, NOOP); + testee.addListener(KEY_1, listener2, NOOP); + testee.addListener(KEY_1, listener3, NOOP); + testee.addListener(KEY_1, listener4, NOOP); + testee.addListener(KEY_1, listener5, NOOP); + + Mono<List<MailboxListener>> listeners = testee.getLocalMailboxListeners(KEY_1) + .publishOn(Schedulers.elastic()) + .delayElements(Duration.ofMillis(100)) + .collectList(); + + testee.removeListener(KEY_1, listener5, NOOP); + + assertThat(listeners.block(Duration.ofSeconds(10))).hasSize(5); + } } } \ No newline at end of file --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
