From d9969cb7d4107c4ffb8480c69ff93193c274fb0e Mon Sep 17 00:00:00 2001
From: mhyeon-lee <mhyeon.lee@navercorp.com>
Date: Wed, 27 May 2020 15:10:25 +0900
Subject: [PATCH] DATACMNS-1735 - Fix ConcurrencyModificationException in
 EntityCallbackDiscoverer.

We now guard modifications of the cached entity callbacks list with a synchronization block.

Original pull request: #446.
---
 .../callback/EntityCallbackDiscoverer.java    | 11 ++++--
 .../EntityCallbackDiscovererUnitTests.java    | 38 +++++++++++++++++++
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/src/main/java/org/springframework/data/mapping/callback/EntityCallbackDiscoverer.java b/src/main/java/org/springframework/data/mapping/callback/EntityCallbackDiscoverer.java
index ad65c6de0..f80a1da22 100644
--- a/src/main/java/org/springframework/data/mapping/callback/EntityCallbackDiscoverer.java
+++ b/src/main/java/org/springframework/data/mapping/callback/EntityCallbackDiscoverer.java
@@ -44,6 +44,7 @@ import org.springframework.util.comparator.Comparators;
 /**
  * @author Mark Paluch
  * @author Christoph Strobl
+ * @author Myeonghyeon Lee
  * @since 2.2
  */
 class EntityCallbackDiscoverer {
@@ -383,9 +384,13 @@ class EntityCallbackDiscoverer {
 			if (this.entityCallbackBeans.isEmpty()) {
 
 				if (cachedEntityCallbacks.size() != entityCallbacks.size()) {
-					cachedEntityCallbacks.clear();
-					cachedEntityCallbacks.addAll(entityCallbacks);
-					AnnotationAwareOrderComparator.sort(cachedEntityCallbacks);
+					List<EntityCallback<?>> entityCallbacks = new ArrayList<>(this.entityCallbacks.size());
+					AnnotationAwareOrderComparator.sort(entityCallbacks);
+
+					synchronized(this) {
+						cachedEntityCallbacks.clear();
+						cachedEntityCallbacks.addAll(entityCallbacks);
+					}
 				}
 
 				return cachedEntityCallbacks;
diff --git a/src/test/java/org/springframework/data/mapping/callback/EntityCallbackDiscovererUnitTests.java b/src/test/java/org/springframework/data/mapping/callback/EntityCallbackDiscovererUnitTests.java
index 17dfd9b1c..5d9c24bca 100644
--- a/src/test/java/org/springframework/data/mapping/callback/EntityCallbackDiscovererUnitTests.java
+++ b/src/test/java/org/springframework/data/mapping/callback/EntityCallbackDiscovererUnitTests.java
@@ -18,6 +18,9 @@ package org.springframework.data.mapping.callback;
 import static org.assertj.core.api.Assertions.*;
 
 import java.util.Collection;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.CountDownLatch;
 
 import org.junit.jupiter.api.Test;
 import org.springframework.context.annotation.AnnotationConfigApplicationContext;
@@ -33,6 +36,7 @@ import org.springframework.data.mapping.PersonDocument;
 
 /**
  * @author Christoph Strobl
+ * @author Myeonghyeon Lee
  */
 class EntityCallbackDiscovererUnitTests {
 
@@ -49,6 +53,40 @@ class EntityCallbackDiscovererUnitTests {
 		assertThat(entityCallbacks).hasSize(1).element(0).isInstanceOf(MyBeforeSaveCallback.class);
 	}
 
+	@Test // DATACMNS-1735
+	void shouldDiscoverCallbackTypeConcurrencyCache() throws InterruptedException {
+
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class);
+
+		EntityCallbackDiscoverer discoverer = new EntityCallbackDiscoverer(ctx);
+
+		int concurrencyCount = 4000;
+		CountDownLatch startLatch = new CountDownLatch(concurrencyCount);
+		CountDownLatch doneLatch = new CountDownLatch(concurrencyCount);
+
+		List<Exception> exceptions = new CopyOnWriteArrayList<>();
+		for (int i = 0; i < concurrencyCount; i++) {
+			Thread thread = new Thread(() -> {
+				try {
+					startLatch.countDown();
+					startLatch.await();
+
+					discoverer.getEntityCallbacks(PersonDocument.class,
+						ResolvableType.forType(BeforeSaveCallback.class));
+				} catch (Exception ex) {
+					exceptions.add(ex);
+				} finally {
+					doneLatch.countDown();
+				}
+			});
+			thread.start();
+		}
+
+		doneLatch.await();
+
+		assertThat(exceptions).isEmpty();
+	}
+
 	@Test // DATACMNS-1467
 	void shouldDiscoverCallbackTypeByName() {