Spring Bean 单例模式

1. Refer to <<Spring in action>>, page 39.

2. Spring Bean Creation is Not Thread Safe

refer to http://blog.imaginea.com/spring-bean-creation-is-not-thread-safe/

When it comes to Java Concurrency no body[except Doug Lea] is sure of correctness of the code. The major problem faced by developers is debugging concurrent bugs. They are not reproducible easily and not debuggable by Remote Java Debug as that can effect the behavior of concurrent program.

In this article I am going to talk about a concurrency issue I faced in Spring. I will run through the steps I used for analyzing it. The conclusion would talk about a thumb rule that we may consider while refactoring concurrent code.

Issue: While using Spring as a bean container in heavy load I was getting some beans which were not initialized completely.

Approach

1. Divide: The first step was to isolate the problem to its smallest set of replication steps. This is real important as unless you can point to very small piece of code which has issue, it will be hard to debug it. So I created two classes say A and B. In Class A I was injecting an instance of B. My Classes looked something like this:

public class A{private B b;//getters and setters for b}public class B{}

I configured these beans in spring as lazy. I then started 1000 threads and all threads invoked getBean on Spring ApplicationContext. Many of the threads got instance of Class “A” with instance variable “b” being null, which means Spring gave me a bean instance which was not fully initialized.

2. Gather: The next step was to gather more information about the variable states. Unfortunately, debugging for concurrency issues has its drawbacks, so I configured logging for Spring.

3. Analyze: I found following statements in log

beans.factory.support.DefaultListableBeanFactory - Creating shared instance of singleton bean ‘A’

beans.factory.support.DefaultListableBeanFactory - Eagerly caching bean 'A' to allow for resolving potential circular references


beans.factory.support.DefaultListableBeanFactory - Returning eagerly cached instance of singleton bean 'A' that is not fully initialized yet - a consequence of a circular reference
beans.factory.support.DefaultListableBeanFactory - Creating shared instance of singleton bean 'B'


4. So just from going through the logs I got a hint that something is wrong with eager caching of Beans.

Lets dive into Spring Code Base and see why i was getting uninitialized Bean “A”
Lets look into AbstractBeanFactory method getBean
Class : org.springframework.beans.factory.support.AbstractBeanFactory
Method : getBean
Code Snippet :

Object sharedInstance = getSingleton(beanName);     if (sharedInstance != null) {       if (logger.isDebugEnabled()) {             if (isSingletonCurrentlyInCreation(beanName)) {                   logger.debug("Returning eagerly cached instance of singleton bean '" + beanName +                   "that is not fully initialized yet - a consequence of a circular reference");               }               else {                   logger.debug("Returning cached instance of singleton bean " + beanName);              }      }     bean = getObjectForBeanInstance(sharedInstance, name, beanName);}

This means Bean A was identified as an eagerly cached instance. Seems like, an eagerly cached instance can be returned before it is completely initialized. Though the code seemed to expect such scenarios only in case of circular references, this certainly wasn’t true in my case [as I don’t have any circular reference in my bean definition].

Lets look at the Locks used in this method:
a.) There is no lock in the method shown above
b.) There are no locks in getSingleton either, this method looks as follows:

public Object getSingleton(String beanName) {      Object singletonObject = this.singletonObjects.get(beanName);      return (singletonObject != NULL_OBJECT ? singletonObject : null);}

there are no explicit locks, only lock taken is in get method on the map singletonObject which happens to be a concurrent Hash Map.

The surprising part is the absence of locks in getSingleton method, a lock here could have stopped cached beans from returning to other threads [as circular reference call will come from same Thread.]?
So is there an issue with locking in getSingleton method?
A possible sequence of steps:
Thread 1: Goes and asks for bean “A”. Spring starts instantiating the bean. Adds it to the singletonObjects map before completely resolving all dependencies of bean.
Thread 2: Goes and asks for bean “A” and till this time Thread 1 has already added it into singletonObjects map but Thread 1 is still resolving dependencies. But now Thread 2 will see an instance of “A” which is not completely initialized.

That’s why Spring Bean Creation is not Thread Safe.

If you dig further you will see that Bean Creation in Spring is guraded by a lock on singletonObjects, but method getSingleton(String beanName) is not guarded by the same lock.
Bean Creation Method in Spring:

public Object getSingleton(String beanName, ObjectFactory singletonFactory) {    Assert.notNull(beanName, "'beanName' must not be null");    synchronized (this.singletonObjects) {

So you can see that Bean Creation is guarded by lock on singletonObjects but getSingleton(String beanName) is not guarded by same lock.
How to Fix it.

public Object getSingleton(String beanName) {     synchronized(this.singletonObjects){           Object singletonObject = this.singletonObjects.get(beanName);           return (singletonObject != NULL_OBJECT ? singletonObject : null);     } }

Now getSingleton(String beanName) and method which creates the Bean both are guarded by the same lock. So now Bean Creation is Thread Safe.

You will find this bug in Spring Version 2.5 and not in 2.0. Why ?
Lets look at Spring 2.0 code :
Class : DefaultSingletonBeanRegistry

Class : DefaultSingletonBeanRegistry public Object getSingleton(String beanName) {      synchronized (this.singletonCache) {         return this.singletonCache.get(beanName);     } }public Object getSingleton(String beanName, ObjectFactory singletonFactory) {     synchronized (this.singletonCache) {     ......

So it works as in this case as both methods are guarded by the same lock.But why in the 2.5 version this thing was not taken care of.
If you look in 2.0 code you will find:

/** Cache of singleton objects: bean name –> bean instance */

private final Map singletonCache = new HashMap();

And in 2.5 code :

/** Cache of singleton objects: bean name –> bean instance */

private final Map singletonObjects = CollectionFactory.createConcurrentMapIfPossible(16);

As can be seen the map used in 2.0 code base was a HashMap and in 2.5 method it was replaced with a ThreadSafe Map.
So the author might have thought he doesn’t need any other lock in :

public Object getSingleton(String beanName) {   Object singletonObject = this.singletonObjects.get(beanName);   return (singletonObject != NULL_OBJECT ? singletonObject : null);}

Because the singleton map was now (in 2.5) Thread Safe.

This is one of the reasons why we should be careful while refactoring concurrent code and we should make sure Locks are same.

And specifically while moving from HashMap to a Thread Safe Map, check earlier lock taken was not guarding other then making the Map Thread Safe. Like in this case Lock was guarding singleton cache also and Map thread safety also.