DATACMNS-1735 - Polishing.

Use mutated object as guard for synchronization. Copy discovered callbacks to cached callbacks.

Reduce concurrency in unit test to reduce test load. Guard synchronization with timeouts.

Original pull request: #446.

src/main/java/org/springframework/data/mapping/callback/EntityCallbackDiscoverer.java

@@ -384,10 +384,11 @@ class EntityCallbackDiscoverer {
 			if (this.entityCallbackBeans.isEmpty()) {
 
 				if (cachedEntityCallbacks.size() != entityCallbacks.size()) {
-					List<EntityCallback<?>> entityCallbacks = new ArrayList<>(this.entityCallbacks.size());
+
+					List<EntityCallback<?>> entityCallbacks = new ArrayList<>(this.entityCallbacks);
 					AnnotationAwareOrderComparator.sort(entityCallbacks);
 
-					synchronized(this) {
+					synchronized (cachedEntityCallbacks) {
 						cachedEntityCallbacks.clear();
 						cachedEntityCallbacks.addAll(entityCallbacks);
 					}

src/test/java/org/springframework/data/mapping/callback/EntityCallbackDiscovererUnitTests.java

@@ -21,6 +21,9 @@ import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.LinkedBlockingDeque;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
 
 import org.junit.jupiter.api.Test;
 import org.springframework.context.annotation.AnnotationConfigApplicationContext;
@@ -35,8 +38,11 @@ import org.springframework.data.mapping.Person;
 import org.springframework.data.mapping.PersonDocument;
 
 /**
+ * Unit tests for {@link EntityCallbackDiscoverer}.
+ *
  * @author Christoph Strobl
  * @author Myeonghyeon Lee
+ * @author Mark Paluch
  */
 class EntityCallbackDiscovererUnitTests {
 
@@ -59,30 +65,31 @@ class EntityCallbackDiscovererUnitTests {
 		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class);
 
 		EntityCallbackDiscoverer discoverer = new EntityCallbackDiscoverer(ctx);
-
+		int poolSize = Runtime.getRuntime().availableProcessors();
-		int concurrencyCount = 4000;
+		ThreadPoolExecutor executor = new ThreadPoolExecutor(poolSize, poolSize, 20, TimeUnit.SECONDS,
-		CountDownLatch startLatch = new CountDownLatch(concurrencyCount);
+				new LinkedBlockingDeque<>());
-		CountDownLatch doneLatch = new CountDownLatch(concurrencyCount);
+		CountDownLatch startLatch = new CountDownLatch(poolSize);
+		CountDownLatch doneLatch = new CountDownLatch(poolSize);
 
 		List<Exception> exceptions = new CopyOnWriteArrayList<>();
-		for (int i = 0; i < concurrencyCount; i++) {
+		for (int i = 0; i < poolSize; i++) {
-			Thread thread = new Thread(() -> {
+			executor.submit(() -> {
 				try {
 					startLatch.countDown();
-					startLatch.await();
+					startLatch.await(5, TimeUnit.SECONDS);
 
 					discoverer.getEntityCallbacks(PersonDocument.class,
-						ResolvableType.forType(BeforeSaveCallback.class));
+							ResolvableType.forType(BeforeSaveCallback.class));
 				} catch (Exception ex) {
 					exceptions.add(ex);
 				} finally {
 					doneLatch.countDown();
 				}
 			});
-			thread.start();
 		}
 
-		doneLatch.await();
+		doneLatch.await(10, TimeUnit.SECONDS);
+		executor.shutdownNow();
 
 		assertThat(exceptions).isEmpty();
 	}