Refine Locking

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-mongodb-parent</artifactId>
-	<version>4.2.0-SNAPSHOT</version>
+	<version>4.2.0-GH-4429-SNAPSHOT</version>
 	<packaging>pom</packaging>
 
 	<name>Spring Data MongoDB</name>

spring-data-mongodb-benchmarks/pom.xml

@@ -7,7 +7,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-mongodb-parent</artifactId>
-		<version>4.2.0-SNAPSHOT</version>
+		<version>4.2.0-GH-4429-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

spring-data-mongodb-distribution/pom.xml

@@ -15,7 +15,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-mongodb-parent</artifactId>
-		<version>4.2.0-SNAPSHOT</version>
+		<version>4.2.0-GH-4429-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

spring-data-mongodb/pom.xml

@@ -13,7 +13,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-mongodb-parent</artifactId>
-		<version>4.2.0-SNAPSHOT</version>
+		<version>4.2.0-GH-4429-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoOperations.java

@@ -18,6 +18,7 @@
 import java.util.Collection;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 import java.util.stream.Stream;
@@ -46,6 +47,7 @@
 import org.springframework.data.mongodb.core.query.Query;
 import org.springframework.data.mongodb.core.query.Update;
 import org.springframework.data.mongodb.core.query.UpdateDefinition;
+import org.springframework.data.mongodb.util.Lock;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
@@ -188,17 +190,18 @@ default SessionScoped withSession(Supplier<ClientSession> sessionProvider) {
 
 		return new SessionScoped() {
 
-			private final Object lock = new Object();
-			private @Nullable ClientSession session = null;
+			private final Lock lock = Lock.of(new ReentrantLock());
+			private @Nullable ClientSession session;
 
 			@Override
 			public <T> T execute(SessionCallback<T> action, Consumer<ClientSession> onComplete) {
 
-				synchronized (lock) {
+				lock.executeWithoutResult(() -> {
+
 					if (session == null) {
 						session = sessionProvider.get();
 					}
-				}
+				});
 
 				try {
 					return action.doInSession(MongoOperations.this.withSession(session));
@@ -943,8 +946,8 @@ default <T> List<T> findDistinct(Query query, String field, String collection, C
 	 * Triggers <a href="https://docs.mongodb.org/manual/reference/method/db.collection.findAndModify/">findAndModify </a>
 	 * to apply provided {@link Update} on documents matching {@link Criteria} of given {@link Query}.
 	 *
-	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an optional
-	 *          fields specification. Must not be {@literal null}.
+	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an
+	 *          optional fields specification. Must not be {@literal null}.
 	 * @param update the {@link UpdateDefinition} to apply on matching documents. Must not be {@literal null}.
 	 * @param entityClass the parametrized type. Must not be {@literal null}.
 	 * @return the converted object that was updated before it was updated or {@literal null}, if not found.
@@ -959,8 +962,8 @@ default <T> List<T> findDistinct(Query query, String field, String collection, C
 	 * Triggers <a href="https://docs.mongodb.org/manual/reference/method/db.collection.findAndModify/">findAndModify </a>
 	 * to apply provided {@link Update} on documents matching {@link Criteria} of given {@link Query}.
 	 *
-	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an optional
-	 *          fields specification. Must not be {@literal null}.
+	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an
+	 *          optional fields specification. Must not be {@literal null}.
 	 * @param update the {@link UpdateDefinition} to apply on matching documents. Must not be {@literal null}.
 	 * @param entityClass the parametrized type. Must not be {@literal null}.
 	 * @param collectionName the collection to query. Must not be {@literal null}.
@@ -977,8 +980,8 @@ default <T> List<T> findDistinct(Query query, String field, String collection, C
 	 * to apply provided {@link Update} on documents matching {@link Criteria} of given {@link Query} taking
 	 * {@link FindAndModifyOptions} into account.
 	 *
-	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an optional
-	 *          fields specification.
+	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an
+	 *          optional fields specification.
 	 * @param update the {@link UpdateDefinition} to apply on matching documents.
 	 * @param options the {@link FindAndModifyOptions} holding additional information.
 	 * @param entityClass the parametrized type.
@@ -997,8 +1000,8 @@ default <T> List<T> findDistinct(Query query, String field, String collection, C
 	 * to apply provided {@link Update} on documents matching {@link Criteria} of given {@link Query} taking
 	 * {@link FindAndModifyOptions} into account.
 	 *
-	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an optional
-	 *          fields specification. Must not be {@literal null}.
+	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an
+	 *          optional fields specification. Must not be {@literal null}.
 	 * @param update the {@link UpdateDefinition} to apply on matching documents. Must not be {@literal null}.
 	 * @param options the {@link FindAndModifyOptions} holding additional information. Must not be {@literal null}.
 	 * @param entityClass the parametrized type. Must not be {@literal null}.
@@ -1023,8 +1026,8 @@ <T> T findAndModify(Query query, UpdateDefinition update, FindAndModifyOptions o
 	 * Options are defaulted to {@link FindAndReplaceOptions#empty()}. <br />
 	 * <strong>NOTE:</strong> The replacement entity must not hold an {@literal id}.
 	 *
-	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an optional
-	 *          fields specification. Must not be {@literal null}.
+	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an
+	 *          optional fields specification. Must not be {@literal null}.
 	 * @param replacement the replacement document. Must not be {@literal null}.
 	 * @return the converted object that was updated or {@literal null}, if not found.
 	 * @throws org.springframework.data.mapping.MappingException if the collection name cannot be
@@ -1044,8 +1047,8 @@ default <T> T findAndReplace(Query query, T replacement) {
 	 * Options are defaulted to {@link FindAndReplaceOptions#empty()}. <br />
 	 * <strong>NOTE:</strong> The replacement entity must not hold an {@literal id}.
 	 *
-	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an optional
-	 *          fields specification. Must not be {@literal null}.
+	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an
+	 *          optional fields specification. Must not be {@literal null}.
 	 * @param replacement the replacement document. Must not be {@literal null}.
 	 * @param collectionName the collection to query. Must not be {@literal null}.
 	 * @return the converted object that was updated or {@literal null}, if not found.
@@ -1063,8 +1066,8 @@ default <T> T findAndReplace(Query query, T replacement, String collectionName)
 	 * taking {@link FindAndReplaceOptions} into account.<br />
 	 * <strong>NOTE:</strong> The replacement entity must not hold an {@literal id}.
 	 *
-	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an optional
-	 *          fields specification. Must not be {@literal null}.
+	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an
+	 *          optional fields specification. Must not be {@literal null}.
 	 * @param replacement the replacement document. Must not be {@literal null}.
 	 * @param options the {@link FindAndModifyOptions} holding additional information. Must not be {@literal null}.
 	 * @return the converted object that was updated or {@literal null}, if not found. Depending on the value of
@@ -1086,8 +1089,8 @@ default <T> T findAndReplace(Query query, T replacement, FindAndReplaceOptions o
 	 * taking {@link FindAndReplaceOptions} into account.<br />
 	 * <strong>NOTE:</strong> The replacement entity must not hold an {@literal id}.
 	 *
-	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an optional
-	 *          fields specification. Must not be {@literal null}.
+	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an
+	 *          optional fields specification. Must not be {@literal null}.
 	 * @param replacement the replacement document. Must not be {@literal null}.
 	 * @param options the {@link FindAndModifyOptions} holding additional information. Must not be {@literal null}.
 	 * @return the converted object that was updated or {@literal null}, if not found. Depending on the value of
@@ -1109,8 +1112,8 @@ default <T> T findAndReplace(Query query, T replacement, FindAndReplaceOptions o
 	 * taking {@link FindAndReplaceOptions} into account.<br />
 	 * <strong>NOTE:</strong> The replacement entity must not hold an {@literal id}.
 	 *
-	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an optional
-	 *          fields specification. Must not be {@literal null}.
+	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an
+	 *          optional fields specification. Must not be {@literal null}.
 	 * @param replacement the replacement document. Must not be {@literal null}.
 	 * @param options the {@link FindAndModifyOptions} holding additional information. Must not be {@literal null}.
 	 * @param entityType the parametrized type. Must not be {@literal null}.
@@ -1134,8 +1137,8 @@ default <T> T findAndReplace(Query query, T replacement, FindAndReplaceOptions o
 	 * taking {@link FindAndReplaceOptions} into account.<br />
 	 * <strong>NOTE:</strong> The replacement entity must not hold an {@literal id}.
 	 *
-	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an optional
-	 *          fields specification. Must not be {@literal null}.
+	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an
+	 *          optional fields specification. Must not be {@literal null}.
 	 * @param replacement the replacement document. Must not be {@literal null}.
 	 * @param options the {@link FindAndModifyOptions} holding additional information. Must not be {@literal null}.
 	 * @param entityType the type used for mapping the {@link Query} to domain type fields and deriving the collection
@@ -1164,8 +1167,8 @@ default <S, T> T findAndReplace(Query query, S replacement, FindAndReplaceOption
 	 * taking {@link FindAndReplaceOptions} into account.<br />
 	 * <strong>NOTE:</strong> The replacement entity must not hold an {@literal id}.
 	 *
-	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an optional
-	 *          fields specification. Must not be {@literal null}.
+	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a document and also an
+	 *          optional fields specification. Must not be {@literal null}.
 	 * @param replacement the replacement document. Must not be {@literal null}.
 	 * @param options the {@link FindAndModifyOptions} holding additional information. Must not be {@literal null}.
 	 * @param entityType the type used for mapping the {@link Query} to domain type fields. Must not be {@literal null}.
@@ -1673,7 +1676,8 @@ default long exactCount(Query query, String collectionName) {
 	 * acknowledged} remove operation was successful or not.
 	 *
 	 * @param object must not be {@literal null}.
-	 * @param collectionName name of the collection where the documents will be removed from, must not be {@literal null} or empty.
+	 * @param collectionName name of the collection where the documents will be removed from, must not be {@literal null}
+	 *          or empty.
 	 * @return the {@link DeleteResult} which lets you access the results of the previous delete.
 	 */
 	DeleteResult remove(Object object, String collectionName);
@@ -1697,7 +1701,8 @@ default long exactCount(Query query, String collectionName) {
 	 *
 	 * @param query the query document that specifies the criteria used to remove a document.
 	 * @param entityClass class of the pojo to be operated on. Can be {@literal null}.
-	 * @param collectionName name of the collection where the documents will be removed from, must not be {@literal null} or empty.
+	 * @param collectionName name of the collection where the documents will be removed from, must not be {@literal null}
+	 *          or empty.
 	 * @return the {@link DeleteResult} which lets you access the results of the previous delete.
 	 * @throws IllegalArgumentException when {@literal query}, {@literal entityClass} or {@literal collectionName} is
 	 *           {@literal null}.
@@ -1711,7 +1716,8 @@ default long exactCount(Query query, String collectionName) {
 	 * information. Use {@link #remove(Query, Class, String)} to get full type specific support.
 	 *
 	 * @param query the query document that specifies the criteria used to remove a document.
-	 * @param collectionName name of the collection where the documents will be removed from, must not be {@literal null} or empty.
+	 * @param collectionName name of the collection where the documents will be removed from, must not be {@literal null}
+	 *          or empty.
 	 * @return the {@link DeleteResult} which lets you access the results of the previous delete.
 	 * @throws IllegalArgumentException when {@literal query} or {@literal collectionName} is {@literal null}.
 	 */
@@ -1723,7 +1729,8 @@ default long exactCount(Query query, String collectionName) {
 	 * information. Use {@link #findAllAndRemove(Query, Class, String)} to get full type specific support.
 	 *
 	 * @param query the query document that specifies the criteria used to find and remove documents.
-	 * @param collectionName name of the collection where the documents will be removed from, must not be {@literal null} or empty.
+	 * @param collectionName name of the collection where the documents will be removed from, must not be {@literal null}
+	 *          or empty.
 	 * @return the {@link List} converted objects deleted by this operation.
 	 * @since 1.5
 	 */
@@ -1748,7 +1755,8 @@ default long exactCount(Query query, String collectionName) {
 	 *
 	 * @param query the query document that specifies the criteria used to find and remove documents.
 	 * @param entityClass class of the pojo to be operated on.
-	 * @param collectionName name of the collection where the documents will be removed from, must not be {@literal null} or empty.
+	 * @param collectionName name of the collection where the documents will be removed from, must not be {@literal null}
+	 *          or empty.
 	 * @return the {@link List} converted objects deleted by this operation.
 	 * @since 1.5
 	 */

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/LazyLoadingProxyFactory.java

@@ -22,6 +22,8 @@
 import java.io.ObjectOutputStream;
 import java.io.Serializable;
 import java.lang.reflect.Method;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.function.Supplier;
 
 import org.aopalliance.intercept.MethodInterceptor;
@@ -39,6 +41,8 @@
 import org.springframework.data.mongodb.ClientSessionException;
 import org.springframework.data.mongodb.LazyLoadingException;
 import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty;
+import org.springframework.data.mongodb.util.Lock;
+import org.springframework.data.mongodb.util.Lock.AcquiredLock;
 import org.springframework.lang.Nullable;
 import org.springframework.objenesis.SpringObjenesis;
 import org.springframework.util.ReflectionUtils;
@@ -134,7 +138,8 @@ public Object createLazyLoadingProxy(MongoPersistentProperty property, DbRefReso
 		}
 
 		return prepareProxyFactory(propertyType,
-				() -> new LazyLoadingInterceptor(property, callback, source, exceptionTranslator)).getProxy(LazyLoadingProxy.class.getClassLoader());
+				() -> new LazyLoadingInterceptor(property, callback, source, exceptionTranslator))
+						.getProxy(LazyLoadingProxy.class.getClassLoader());
 	}
 
 	/**
@@ -171,6 +176,10 @@ public static class LazyLoadingInterceptor
 			}
 		}
 
+		private final ReadWriteLock rwLock = new ReentrantReadWriteLock();
+		private final Lock readLock = Lock.of(rwLock.readLock());
+		private final Lock writeLock = Lock.of(rwLock.writeLock());
+
 		private final MongoPersistentProperty property;
 		private final DbRefResolverCallback callback;
 		private final Object source;
@@ -339,25 +348,26 @@ private void readObject(ObjectInputStream in) throws IOException {
 		}
 
 		@Nullable
-		private synchronized Object resolve() {
+		private Object resolve() {
 
-			if (resolved) {
+			try (AcquiredLock l = readLock.lock()) {
+				if (resolved) {
 
-				if (LOGGER.isTraceEnabled()) {
-					LOGGER.trace(String.format("Accessing already resolved lazy loading property %s.%s",
-							property.getOwner() != null ? property.getOwner().getName() : "unknown", property.getName()));
+					if (LOGGER.isTraceEnabled()) {
+						LOGGER.trace(String.format("Accessing already resolved lazy loading property %s.%s",
+								property.getOwner() != null ? property.getOwner().getName() : "unknown", property.getName()));
+					}
+					return result;
 				}
-				return result;
 			}
 
-			try {
-				if (LOGGER.isTraceEnabled()) {
-					LOGGER.trace(String.format("Resolving lazy loading property %s.%s",
-							property.getOwner() != null ? property.getOwner().getName() : "unknown", property.getName()));
-				}
-
-				return callback.resolve(property);
+			if (LOGGER.isTraceEnabled()) {
+				LOGGER.trace(String.format("Resolving lazy loading property %s.%s",
+						property.getOwner() != null ? property.getOwner().getName() : "unknown", property.getName()));
+			}
 
+			try {
+				return writeLock.execute(() -> callback.resolve(property));
 			} catch (RuntimeException ex) {
 
 				DataAccessException translatedException = exceptionTranslator.translateExceptionIfPossible(ex);
@@ -370,6 +380,7 @@ private synchronized Object resolve() {
 						translatedException != null ? translatedException : ex);
 			}
 		}
+
 	}
 
 }