Skip to content

Commit

Permalink
Apply lenient locking fallback to singleton pre-instantiation phase only
Browse files Browse the repository at this point in the history
Closes gh-33463
  • Loading branch information
jhoeller committed Sep 11, 2024
1 parent 1ff2678 commit a044357
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,8 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto
/** Whether bean definition metadata may be cached for all beans. */
private volatile boolean configurationFrozen;

private volatile boolean preInstantiationPhase;

private final NamedThreadLocal<PreInstantiation> preInstantiationThread =
new NamedThreadLocal<>("Pre-instantiation thread marker");

Expand Down Expand Up @@ -1001,8 +1003,9 @@ protected void checkMergedBeanDefinition(RootBeanDefinition mbd, String beanName
}

@Override
protected boolean isCurrentThreadAllowedToHoldSingletonLock() {
return (this.preInstantiationThread.get() != PreInstantiation.BACKGROUND);
@Nullable
protected Boolean isCurrentThreadAllowedToHoldSingletonLock() {
return (this.preInstantiationPhase ? this.preInstantiationThread.get() != PreInstantiation.BACKGROUND : null);
}

@Override
Expand All @@ -1017,6 +1020,8 @@ public void preInstantiateSingletons() throws BeansException {

// Trigger initialization of all non-lazy singleton beans...
List<CompletableFuture<?>> futures = new ArrayList<>();

this.preInstantiationPhase = true;
this.preInstantiationThread.set(PreInstantiation.MAIN);
try {
for (String beanName : beanNames) {
Expand All @@ -1031,7 +1036,9 @@ public void preInstantiateSingletons() throws BeansException {
}
finally {
this.preInstantiationThread.remove();
this.preInstantiationPhase = false;
}

if (!futures.isEmpty()) {
try {
CompletableFuture.allOf(futures.toArray(new CompletableFuture<?>[0])).join();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,6 @@ public class DefaultSingletonBeanRegistry extends SimpleAliasRegistry implements
/** Names of beans currently excluded from in creation checks. */
private final Set<String> inCreationCheckExclusions = ConcurrentHashMap.newKeySet(16);

@Nullable
private volatile Thread singletonCreationThread;

/** Flag that indicates whether we're currently within destroySingletons. */
private volatile boolean singletonsCurrentlyInDestruction = false;

Expand Down Expand Up @@ -242,37 +239,33 @@ protected Object getSingleton(String beanName, boolean allowEarlyReference) {
public Object getSingleton(String beanName, ObjectFactory<?> singletonFactory) {
Assert.notNull(beanName, "Bean name must not be null");

boolean acquireLock = isCurrentThreadAllowedToHoldSingletonLock();
Boolean lockFlag = isCurrentThreadAllowedToHoldSingletonLock();
boolean acquireLock = !Boolean.FALSE.equals(lockFlag);
boolean locked = (acquireLock && this.singletonLock.tryLock());
try {
Object singletonObject = this.singletonObjects.get(beanName);
if (singletonObject == null) {
if (acquireLock) {
if (locked) {
this.singletonCreationThread = Thread.currentThread();
if (acquireLock && !locked) {
if (Boolean.TRUE.equals(lockFlag)) {
// Another thread is busy in a singleton factory callback, potentially blocked.
// Fallback as of 6.2: process given singleton bean outside of singleton lock.
// 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 \"" +
Thread.currentThread().getName() + "\" while other thread holds " +
"singleton lock for other beans " + this.singletonsCurrentlyInCreation);
}
}
else {
Thread threadWithLock = this.singletonCreationThread;
if (threadWithLock != null) {
// Another thread is busy in a singleton factory callback, potentially blocked.
// Fallback as of 6.2: process given singleton bean outside of singleton lock.
// 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 \"" +
Thread.currentThread().getName() + "\" while thread \"" + threadWithLock.getName() +
"\" holds singleton lock for other beans " + this.singletonsCurrentlyInCreation);
}
}
else {
// Singleton lock currently held by some other registration method -> wait.
this.singletonLock.lock();
locked = true;
// Singleton object might have possibly appeared in the meantime.
singletonObject = this.singletonObjects.get(beanName);
if (singletonObject != null) {
return singletonObject;
}
// No specific locking indication (outside a coordinated bootstrap) and
// singleton lock currently held by some other creation method -> wait.
this.singletonLock.lock();
locked = true;
// Singleton object might have possibly appeared in the meantime.
singletonObject = this.singletonObjects.get(beanName);
if (singletonObject != null) {
return singletonObject;
}
}
}
Expand All @@ -291,7 +284,6 @@ public Object getSingleton(String beanName, ObjectFactory<?> singletonFactory) {
if (recordSuppressedExceptions) {
this.suppressedExceptions = new LinkedHashSet<>();
}
this.singletonCreationThread = Thread.currentThread();
try {
singletonObject = singletonFactory.getObject();
newSingleton = true;
Expand All @@ -313,7 +305,6 @@ public Object getSingleton(String beanName, ObjectFactory<?> singletonFactory) {
throw ex;
}
finally {
this.singletonCreationThread = null;
if (recordSuppressedExceptions) {
this.suppressedExceptions = null;
}
Expand All @@ -336,10 +327,15 @@ public Object getSingleton(String beanName, ObjectFactory<?> singletonFactory) {
* 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)
* @since 6.2
*/
protected boolean isCurrentThreadAllowedToHoldSingletonLock() {
return true;
@Nullable
protected Boolean isCurrentThreadAllowedToHoldSingletonLock() {
return null;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ void fallbackForThreadDuringInitialization() {
new RootBeanDefinition(ThreadDuringInitialization.class));
beanFactory.registerBeanDefinition("bean2",
new RootBeanDefinition(TestBean.class, () -> new TestBean("tb")));
beanFactory.getBean(ThreadDuringInitialization.class);
beanFactory.preInstantiateSingletons();
}


Expand Down

0 comments on commit a044357

Please sign in to comment.