From 154794f91bd76f87e11619168448a852bd0b34b8 Mon Sep 17 00:00:00 2001
From: Oliver Gierke <info@olivergierke.de>
Date: Tue, 23 Aug 2011 20:12:54 +0200
Subject: [PATCH] DATACMNS-65 - Using ReentrantReadWriteLock to prevent race
 conditions in PersistentEntity creation.

We have to lock PersistentEntity creation as instances could be prematurely handed out and iteration over the properties might interfere new properties being added to the Set iterated over.
---
 .../context/AbstractMappingContext.java       |  36 +++++-
 ...AbstractMappingContextIntegrationTest.java | 109 ++++++++++++++++++
 2 files changed, 140 insertions(+), 5 deletions(-)
 create mode 100644 spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextIntegrationTest.java

diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java b/spring-data-commons-core/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java
index 027917954..52b6edb99 100644
--- a/spring-data-commons-core/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java
+++ b/spring-data-commons-core/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java
@@ -32,6 +32,8 @@ import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.context.ApplicationEventPublisher;
@@ -51,6 +53,9 @@ import org.springframework.validation.Validator;
 /**
  * Base class to build mapping metadata and thus create instances of {@link PersistentEntity} and
  * {@link PersistentProperty}.
+ * <p>
+ * The implementation uses a {@link ReentrantReadWriteLock} to make sure {@link PersistentEntity} are completely
+ * populated before accessing them from outside.
  * 
  * @param E the concrete {@link PersistentEntity} type the {@link MappingContext} implementation creates
  * @param P the concrete {@link PersistentProperty} type the {@link MappingContext} implementation creates
@@ -70,6 +75,10 @@ public abstract class AbstractMappingContext<E extends MutablePersistentEntity<?
 	private boolean strict = false;
 	private SimpleTypeHolder simpleTypeHolder = new SimpleTypeHolder();
 
+	private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
+	private final Lock read = lock.readLock();
+	private final Lock write = lock.writeLock();
+
 	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.context.ApplicationEventPublisherAware#setApplicationEventPublisher(org.springframework.context.ApplicationEventPublisher)
@@ -115,7 +124,12 @@ public abstract class AbstractMappingContext<E extends MutablePersistentEntity<?
 	 * @see org.springframework.data.mapping.model.MappingContext#getPersistentEntities()
 	 */
 	public Collection<E> getPersistentEntities() {
-		return persistentEntities.values();
+		try {
+			read.lock();
+			return persistentEntities.values();
+		} finally {
+			read.unlock();
+		}
 	}
 
 	/*
@@ -132,10 +146,16 @@ public abstract class AbstractMappingContext<E extends MutablePersistentEntity<?
 	 */
 	public E getPersistentEntity(TypeInformation<?> type) {
 
-		E entity = persistentEntities.get(type);
+		try {
+			read.lock();
+			E entity = persistentEntities.get(type);
 
-		if (entity != null) {
-			return entity;
+			if (entity != null) {
+				return entity;
+			}
+
+		} finally {
+			read.unlock();
 		}
 
 		if (strict) {
@@ -144,7 +164,7 @@ public abstract class AbstractMappingContext<E extends MutablePersistentEntity<?
 
 		return addPersistentEntity(type);
 	}
-	
+
 	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.data.mapping.context.MappingContext#getPersistentPropertyPath(java.lang.Class, java.lang.String)
@@ -206,6 +226,9 @@ public abstract class AbstractMappingContext<E extends MutablePersistentEntity<?
 		Class<?> type = typeInformation.getType();
 
 		try {
+
+			write.lock();
+
 			final E entity = createPersistentEntity(typeInformation);
 
 			// Eagerly cache the entity as we might have to find it during recursive lookups.
@@ -260,8 +283,11 @@ public abstract class AbstractMappingContext<E extends MutablePersistentEntity<?
 			}
 
 			return entity;
+
 		} catch (IntrospectionException e) {
 			throw new MappingException(e.getMessage(), e);
+		} finally {
+			write.unlock();
 		}
 	}
 
diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextIntegrationTest.java b/spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextIntegrationTest.java
new file mode 100644
index 000000000..bf66f5203
--- /dev/null
+++ b/spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextIntegrationTest.java
@@ -0,0 +1,109 @@
+/*
+ * Copyright (c) 2011 by the original author(s).
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.mapping.context;
+
+import static org.mockito.Mockito.*;
+
+import java.beans.PropertyDescriptor;
+import java.lang.reflect.Field;
+
+import org.junit.Test;
+import org.springframework.data.mapping.PersistentEntity;
+import org.springframework.data.mapping.PersistentProperty;
+import org.springframework.data.mapping.PropertyHandler;
+import org.springframework.data.mapping.model.BasicPersistentEntity;
+import org.springframework.data.mapping.model.SimpleTypeHolder;
+import org.springframework.data.util.TypeInformation;
+
+/**
+ * Unit tests for {@link AbstractMappingContext}.
+ * 
+ * @author Oliver Gierke
+ */
+public class AbstractMappingContextIntegrationTest<T extends PersistentProperty<T>> {
+
+	@Test
+	public void foo() throws InterruptedException {
+
+		final DummyMappingContext context = new DummyMappingContext();
+
+		Thread a = new Thread(new Runnable() {
+			public void run() {
+				context.getPersistentEntity(Person.class);
+			}
+		});
+
+		Thread b = new Thread(new Runnable() {
+
+			public void run() {
+
+				PersistentEntity<Object, T> entity = context.getPersistentEntity(Person.class);
+				
+				entity.doWithProperties(new PropertyHandler<T>() {
+					public void doWithPersistentProperty(T persistentProperty) {
+						try {
+							Thread.sleep(1000);
+						} catch (InterruptedException e) {
+							throw new RuntimeException(e);
+						}
+					}
+				});
+			}
+		});
+
+		a.start();
+		Thread.sleep(2800);
+		b.start();
+		
+		a.join();
+		b.join();
+	}
+
+	class DummyMappingContext extends AbstractMappingContext<BasicPersistentEntity<Object, T>, T> {
+
+		@Override
+		@SuppressWarnings("unchecked")
+		protected <S> BasicPersistentEntity<Object, T> createPersistentEntity(TypeInformation<S> typeInformation) {
+			return (BasicPersistentEntity<Object, T>) new BasicPersistentEntity<S, T>(typeInformation);
+		}
+
+		@Override
+		@SuppressWarnings({ "rawtypes", "unchecked" })
+		protected T createPersistentProperty(final Field field, final PropertyDescriptor descriptor,
+				final BasicPersistentEntity<Object, T> owner, final SimpleTypeHolder simpleTypeHolder) {
+
+			PersistentProperty prop = mock(PersistentProperty.class);
+			
+			when(prop.getTypeInformation()).thenReturn((TypeInformation) owner.getTypeInformation());
+			when(prop.getName()).thenReturn(field.getName());
+
+			try {
+				Thread.sleep(800);
+			} catch (InterruptedException e) {
+				throw new RuntimeException(e);
+			}
+			
+			return (T) prop;
+		}
+	}
+
+	class Person {
+
+		String firstname;
+		String lastname;
+		String email;
+	}
+}