Efficient concurrency in MethodOverrides through CopyOnWriteArraySet

Also restores immediate MethodOverrides instance in AbstractBeanDefinition, avoiding potential lazy-init race condition.

Closes gh-23448

Author: jhoeller

spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanDefinition.java

@@ -179,8 +179,7 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 	@Nullable
 	private MutablePropertyValues propertyValues;
 
-	@Nullable
-	private MethodOverrides methodOverrides;
+	private MethodOverrides methodOverrides = new MethodOverrides();
 
 	@Nullable
 	private String initMethodName;
@@ -869,9 +868,6 @@ public void setMethodOverrides(MethodOverrides methodOverrides) {
 	 * <p>Never returns {@code null}.
 	 */
 	public MethodOverrides getMethodOverrides() {
-		if (this.methodOverrides == null) {
-			this.methodOverrides = new MethodOverrides();
-		}
 		return this.methodOverrides;
 	}
 
@@ -880,7 +876,7 @@ public MethodOverrides getMethodOverrides() {
 	 * @since 5.0.2
 	 */
 	public boolean hasMethodOverrides() {
-		return (this.methodOverrides != null && !this.methodOverrides.isEmpty());
+		return !this.methodOverrides.isEmpty();
 	}
 
 	/**
@@ -1058,10 +1054,9 @@ public BeanDefinition getOriginatingBeanDefinition() {
 	public void validate() throws BeanDefinitionValidationException {
 		if (hasMethodOverrides() && getFactoryMethodName() != null) {
 			throw new BeanDefinitionValidationException(
-					"Cannot combine static factory method with method overrides: " +
-					"the static factory method must create the instance");
+					"Cannot combine factory method with container-generated method overrides: " +
+					"the factory method must create the concrete bean instance.");
 		}
-
 		if (hasBeanClass()) {
 			prepareMethodOverrides();
 		}
@@ -1073,14 +1068,9 @@ public void validate() throws BeanDefinitionValidationException {
 	 * @throws BeanDefinitionValidationException in case of validation failure
 	 */
 	public void prepareMethodOverrides() throws BeanDefinitionValidationException {
-		// Check that lookup methods exists.
+		// Check that lookup methods exist and determine their overloaded status.
 		if (hasMethodOverrides()) {
-			Set<MethodOverride> overrides = getMethodOverrides().getOverrides();
-			synchronized (overrides) {
-				for (MethodOverride mo : overrides) {
-					prepareMethodOverride(mo);
-				}
-			}
+			getMethodOverrides().getOverrides().forEach(this::prepareMethodOverride);
 		}
 	}
 

spring-beans/src/main/java/org/springframework/beans/factory/support/MethodOverrides.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2019 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,9 +17,8 @@
 package org.springframework.beans.factory.support;
 
 import java.lang.reflect.Method;
-import java.util.Collections;
-import java.util.LinkedHashSet;
 import java.util.Set;
+import java.util.concurrent.CopyOnWriteArraySet;
 
 import org.springframework.lang.Nullable;
 
@@ -37,9 +36,7 @@
  */
 public class MethodOverrides {
 
-	private final Set<MethodOverride> overrides = Collections.synchronizedSet(new LinkedHashSet<>(2));
-
-	private volatile boolean modified = false;
+	private final Set<MethodOverride> overrides = new CopyOnWriteArraySet<>();
 
 
 	/**
@@ -61,7 +58,6 @@ public MethodOverrides(MethodOverrides other) {
 	 */
 	public void addOverrides(@Nullable MethodOverrides other) {
 		if (other != null) {
-			this.modified = true;
 			this.overrides.addAll(other.overrides);
 		}
 	}
@@ -70,25 +66,23 @@ public void addOverrides(@Nullable MethodOverrides other) {
 	 * Add the given method override.
 	 */
 	public void addOverride(MethodOverride override) {
-		this.modified = true;
 		this.overrides.add(override);
 	}
 
 	/**
 	 * Return all method overrides contained by this object.
-	 * @return Set of MethodOverride objects
+	 * @return a Set of MethodOverride objects
 	 * @see MethodOverride
 	 */
 	public Set<MethodOverride> getOverrides() {
-		this.modified = true;
 		return this.overrides;
 	}
 
 	/**
 	 * Return whether the set of method overrides is empty.
 	 */
 	public boolean isEmpty() {
-		return (!this.modified || this.overrides.isEmpty());
+		return this.overrides.isEmpty();
 	}
 
 	/**
@@ -98,18 +92,13 @@ public boolean isEmpty() {
 	 */
 	@Nullable
 	public MethodOverride getOverride(Method method) {
-		if (!this.modified) {
-			return null;
-		}
-		synchronized (this.overrides) {
-			MethodOverride match = null;
-			for (MethodOverride candidate : this.overrides) {
-				if (candidate.matches(method)) {
-					match = candidate;
-				}
+		MethodOverride match = null;
+		for (MethodOverride candidate : this.overrides) {
+			if (candidate.matches(method)) {
+				match = candidate;
 			}
-			return match;
 		}
+		return match;
 	}
 
 
@@ -123,7 +112,6 @@ public boolean equals(Object other) {
 		}
 		MethodOverrides that = (MethodOverrides) other;
 		return this.overrides.equals(that.overrides);
-
 	}
 
 	@Override