Efficient concurrency in MethodOverrides through CopyOnWriteArraySet

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

Closes gh-23448
This commit is contained in:
Juergen Hoeller 2019-09-25 12:09:16 +02:00
parent 20fc7e178a
commit da44a247cb
2 changed files with 15 additions and 37 deletions

View File

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

View File

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