Browse Source

Avoid lenient locking for additional external bootstrap threads

Includes spring.locking.strict revision to differentiate between true, false, not set.
Includes checkFlag accessor on SpringProperties, also used in StatementCreatorUtils.

Closes gh-34729
See gh-34303
pull/35405/head
Juergen Hoeller 8 months ago
parent
commit
eea6addd26
  1. 47
      spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java
  2. 18
      spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java
  3. 84
      spring-context/src/test/java/org/springframework/context/annotation/BackgroundBootstrapTests.java
  4. 28
      spring-core/src/main/java/org/springframework/core/SpringProperties.java
  5. 9
      spring-jdbc/src/main/java/org/springframework/jdbc/core/StatementCreatorUtils.java

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

@ -133,6 +133,11 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto @@ -133,6 +133,11 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto
* System property that instructs Spring to enforce strict locking during bean creation,
* rather than the mix of strict and lenient locking that 6.2 applies by default. Setting
* this flag to "true" restores 6.1.x style locking in the entire pre-instantiation phase.
* <p>By default, the factory infers strict locking from the encountered thread names:
* If additional threads have names that match the thread prefix of the main bootstrap thread,
* they are considered external (multiple external bootstrap threads calling into the factory)
* and therefore have strict locking applied to them. This inference can be turned off through
* explicitly setting this flag to "false" rather than leaving it unspecified.
* @since 6.2.6
* @see #preInstantiateSingletons()
*/
@ -157,8 +162,9 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto @@ -157,8 +162,9 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto
private static final Map<String, Reference<DefaultListableBeanFactory>> serializableFactories =
new ConcurrentHashMap<>(8);
/** Whether lenient locking is allowed in this factory. */
private final boolean lenientLockingAllowed = !SpringProperties.getFlag(STRICT_LOCKING_PROPERTY_NAME);
/** Whether strict locking is enforced or relaxed in this factory. */
@Nullable
private final Boolean strictLocking = SpringProperties.checkFlag(STRICT_LOCKING_PROPERTY_NAME);
/** Optional id for this factory, for serialization purposes. */
@Nullable
@ -214,6 +220,9 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto @@ -214,6 +220,9 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto
private volatile boolean preInstantiationPhase;
@Nullable
private volatile String mainThreadPrefix;
private final NamedThreadLocal<PreInstantiation> preInstantiationThread =
new NamedThreadLocal<>("Pre-instantiation thread marker");
@ -1045,7 +1054,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto @@ -1045,7 +1054,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto
}
}
else {
// Bean intended to be initialized in main bootstrap thread
// Bean intended to be initialized in main bootstrap thread.
if (this.preInstantiationThread.get() == PreInstantiation.BACKGROUND) {
throw new BeanCurrentlyInCreationException(beanName, "Bean marked for mainline initialization " +
"but requested in background thread - enforce early instantiation in mainline thread " +
@ -1057,8 +1066,28 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto @@ -1057,8 +1066,28 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto
@Override
@Nullable
protected Boolean isCurrentThreadAllowedToHoldSingletonLock() {
return (this.lenientLockingAllowed && this.preInstantiationPhase ?
this.preInstantiationThread.get() != PreInstantiation.BACKGROUND : null);
if (this.preInstantiationPhase) {
// We only differentiate in the preInstantiateSingletons phase.
PreInstantiation preInstantiation = this.preInstantiationThread.get();
if (preInstantiation != null) {
// A Spring-managed thread:
// MAIN is allowed to lock (true) or even forced to lock (null),
// BACKGROUND is never allowed to lock (false).
return switch (preInstantiation) {
case MAIN -> (Boolean.TRUE.equals(this.strictLocking) ? null : true);
case BACKGROUND -> false;
};
}
if (Boolean.FALSE.equals(this.strictLocking) ||
(this.strictLocking == null && !getThreadNamePrefix().equals(this.mainThreadPrefix))) {
// An unmanaged thread (assumed to be application-internal) with lenient locking,
// and not part of the same thread pool that provided the main bootstrap thread
// (excluding scenarios where we are hit by multiple external bootstrap threads).
return true;
}
}
// Traditional behavior: forced to always hold a full lock.
return null;
}
@Override
@ -1076,6 +1105,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto @@ -1076,6 +1105,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto
this.preInstantiationPhase = true;
this.preInstantiationThread.set(PreInstantiation.MAIN);
this.mainThreadPrefix = getThreadNamePrefix();
try {
for (String beanName : beanNames) {
RootBeanDefinition mbd = getMergedLocalBeanDefinition(beanName);
@ -1088,6 +1118,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto @@ -1088,6 +1118,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto
}
}
finally {
this.mainThreadPrefix = null;
this.preInstantiationThread.remove();
this.preInstantiationPhase = false;
}
@ -1183,6 +1214,12 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto @@ -1183,6 +1214,12 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto
}
}
private static String getThreadNamePrefix() {
String name = Thread.currentThread().getName();
int numberSeparator = name.lastIndexOf('-');
return (numberSeparator >= 0 ? name.substring(0, numberSeparator) : name);
}
//---------------------------------------------------------------------
// Implementation of BeanDefinitionRegistry interface

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

@ -272,7 +272,7 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements @@ -272,7 +272,7 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
// Thread-safe exposure is still guaranteed, there is just a risk of collisions
// when triggering creation of other beans as dependencies of the current bean.
if (logger.isInfoEnabled()) {
logger.info("Creating singleton bean '" + beanName + "' in thread \"" +
logger.info("Obtaining singleton bean '" + beanName + "' in thread \"" +
Thread.currentThread().getName() + "\" while other thread holds " +
"singleton lock for other beans " + this.singletonsCurrentlyInCreation);
}
@ -443,12 +443,16 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements @@ -443,12 +443,16 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
/**
* Determine whether the current thread is allowed to hold the singleton lock.
* <p>By default, any thread may acquire and hold the singleton lock, except
* background threads from {@link DefaultListableBeanFactory#setBootstrapExecutor}.
* @return {@code false} if the current thread is explicitly not allowed to hold
* the lock, {@code true} if it is explicitly allowed to hold the lock but also
* accepts lenient fallback behavior, or {@code null} if there is no specific
* indication (traditional behavior: always holding a full lock)
* <p>By default, all threads are forced to hold a full lock through {@code null}.
* {@link DefaultListableBeanFactory} overrides this to specifically handle its
* threads during the pre-instantiation phase: {@code true} for the main thread,
* {@code false} for managed background threads, and configuration-dependent
* behavior for unmanaged threads.
* @return {@code true} if the current thread is explicitly allowed to hold the
* lock but also accepts lenient fallback behavior, {@code false} if it is
* explicitly not allowed to hold the lock and therefore forced to use lenient
* fallback behavior, or {@code null} if there is no specific indication
* (traditional behavior: forced to always hold a full lock)
* @since 6.2
*/
@Nullable

84
spring-context/src/test/java/org/springframework/context/annotation/BackgroundBootstrapTests.java

@ -16,6 +16,9 @@ @@ -16,6 +16,9 @@
package org.springframework.context.annotation;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
@ -67,7 +70,7 @@ class BackgroundBootstrapTests { @@ -67,7 +70,7 @@ class BackgroundBootstrapTests {
@Test
@Timeout(10)
@EnabledForTestGroups(LONG_RUNNING)
void bootstrapWithStrictLockingThread() {
void bootstrapWithStrictLockingFlag() {
SpringProperties.setFlag(DefaultListableBeanFactory.STRICT_LOCKING_PROPERTY_NAME);
try {
ConfigurableApplicationContext ctx = new AnnotationConfigApplicationContext(StrictLockingBeanConfig.class);
@ -79,6 +82,42 @@ class BackgroundBootstrapTests { @@ -79,6 +82,42 @@ class BackgroundBootstrapTests {
}
}
@Test
@Timeout(10)
@EnabledForTestGroups(LONG_RUNNING)
void bootstrapWithStrictLockingInferred() throws InterruptedException {
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
ctx.register(InferredLockingBeanConfig.class);
ExecutorService threadPool = Executors.newFixedThreadPool(2);
threadPool.submit(() -> ctx.refresh());
Thread.sleep(500);
threadPool.submit(() -> ctx.getBean("testBean2"));
Thread.sleep(1000);
assertThat(ctx.getBean("testBean2", TestBean.class).getSpouse()).isSameAs(ctx.getBean("testBean1"));
ctx.close();
}
@Test
@Timeout(10)
@EnabledForTestGroups(LONG_RUNNING)
void bootstrapWithStrictLockingTurnedOff() throws InterruptedException {
SpringProperties.setFlag(DefaultListableBeanFactory.STRICT_LOCKING_PROPERTY_NAME, false);
try {
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
ctx.register(InferredLockingBeanConfig.class);
ExecutorService threadPool = Executors.newFixedThreadPool(2);
threadPool.submit(() -> ctx.refresh());
Thread.sleep(500);
threadPool.submit(() -> ctx.getBean("testBean2"));
Thread.sleep(1000);
assertThat(ctx.getBean("testBean2", TestBean.class).getSpouse()).isNull();
ctx.close();
}
finally {
SpringProperties.setProperty(DefaultListableBeanFactory.STRICT_LOCKING_PROPERTY_NAME, null);
}
}
@Test
@Timeout(10)
@EnabledForTestGroups(LONG_RUNNING)
@ -128,6 +167,24 @@ class BackgroundBootstrapTests { @@ -128,6 +167,24 @@ class BackgroundBootstrapTests {
ctx.close();
}
@Test
@Timeout(10)
@EnabledForTestGroups(LONG_RUNNING)
void bootstrapWithCustomExecutorAndStrictLocking() {
SpringProperties.setFlag(DefaultListableBeanFactory.STRICT_LOCKING_PROPERTY_NAME);
try {
ConfigurableApplicationContext ctx = new AnnotationConfigApplicationContext(CustomExecutorBeanConfig.class);
ctx.getBean("testBean1", TestBean.class);
ctx.getBean("testBean2", TestBean.class);
ctx.getBean("testBean3", TestBean.class);
ctx.getBean("testBean4", TestBean.class);
ctx.close();
}
finally {
SpringProperties.setProperty(DefaultListableBeanFactory.STRICT_LOCKING_PROPERTY_NAME, null);
}
}
@Configuration(proxyBeanMethods = false)
static class UnmanagedThreadBeanConfig {
@ -220,6 +277,27 @@ class BackgroundBootstrapTests { @@ -220,6 +277,27 @@ class BackgroundBootstrapTests {
}
@Configuration(proxyBeanMethods = false)
static class InferredLockingBeanConfig {
@Bean
public TestBean testBean1() {
try {
Thread.sleep(1000);
}
catch (InterruptedException ex) {
Thread.currentThread().interrupt();
}
return new TestBean("testBean1");
}
@Bean
public TestBean testBean2(ConfigurableListableBeanFactory beanFactory) {
return new TestBean((TestBean) beanFactory.getSingleton("testBean1"));
}
}
@Configuration(proxyBeanMethods = false)
static class CircularReferenceAgainstMainThreadBeanConfig {
@ -377,13 +455,13 @@ class BackgroundBootstrapTests { @@ -377,13 +455,13 @@ class BackgroundBootstrapTests {
@Bean(bootstrap = BACKGROUND) @DependsOn("testBean3")
public TestBean testBean1(TestBean testBean3) throws InterruptedException {
Thread.sleep(3000);
Thread.sleep(6000);
return new TestBean();
}
@Bean(bootstrap = BACKGROUND) @Lazy
public TestBean testBean2() throws InterruptedException {
Thread.sleep(3000);
Thread.sleep(6000);
return new TestBean();
}

28
spring-core/src/main/java/org/springframework/core/SpringProperties.java

@ -118,7 +118,18 @@ public final class SpringProperties { @@ -118,7 +118,18 @@ public final class SpringProperties {
* @param key the property key
*/
public static void setFlag(String key) {
localProperties.put(key, Boolean.TRUE.toString());
localProperties.setProperty(key, Boolean.TRUE.toString());
}
/**
* Programmatically set a local flag to the given value, overriding
* an entry in the {@code spring.properties} file (if any).
* @param key the property key
* @param value the associated boolean value
* @since 6.2.6
*/
public static void setFlag(String key, boolean value) {
localProperties.setProperty(key, Boolean.toString(value));
}
/**
@ -131,4 +142,19 @@ public final class SpringProperties { @@ -131,4 +142,19 @@ public final class SpringProperties {
return Boolean.parseBoolean(getProperty(key));
}
/**
* Retrieve the flag for the given property key, returning {@code null}
* instead of {@code false} in case of no actual flag set.
* @param key the property key
* @return {@code true} if the property is set to the string "true"
* (ignoring case), {@code} false if it is set to any other value,
* {@code null} if it is not set at all
* @since 6.2.6
*/
@Nullable
public static Boolean checkFlag(String key) {
String flag = getProperty(key);
return (flag != null ? Boolean.valueOf(flag) : null);
}
}

9
spring-jdbc/src/main/java/org/springframework/jdbc/core/StatementCreatorUtils.java

@ -1,5 +1,5 @@ @@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 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.
@ -86,7 +86,7 @@ public abstract class StatementCreatorUtils { @@ -86,7 +86,7 @@ public abstract class StatementCreatorUtils {
private static final Map<Class<?>, Integer> javaTypeToSqlTypeMap = new HashMap<>(64);
@Nullable
static Boolean shouldIgnoreGetParameterType;
static Boolean shouldIgnoreGetParameterType = SpringProperties.checkFlag(IGNORE_GETPARAMETERTYPE_PROPERTY_NAME);
static {
javaTypeToSqlTypeMap.put(boolean.class, Types.BOOLEAN);
@ -115,11 +115,6 @@ public abstract class StatementCreatorUtils { @@ -115,11 +115,6 @@ public abstract class StatementCreatorUtils {
javaTypeToSqlTypeMap.put(java.sql.Timestamp.class, Types.TIMESTAMP);
javaTypeToSqlTypeMap.put(Blob.class, Types.BLOB);
javaTypeToSqlTypeMap.put(Clob.class, Types.CLOB);
String flag = SpringProperties.getProperty(IGNORE_GETPARAMETERTYPE_PROPERTY_NAME);
if (flag != null) {
shouldIgnoreGetParameterType = Boolean.valueOf(flag);
}
}

Loading…
Cancel
Save