CODEWITHSUNDEEP
javamultithreadingspring
thatidiotguy
thatidiotguy

Reputation: 8991

Spring Singleton Thread Safety

If I have a Java class defined below that is injected in my web application via dependency injection:

public AccountDao
{
   private NamedParameterJdbcTemplate njt;
   private List<Account> accounts;

   public AccountDao(Datasource ds)
   {
       this.njt = new NamedParameterJdbcTemplate(ds);
       refreshAccounts();
   }

   /*called at creation, and then via API calls to inform service new users have 
     been added to the database by a separate program*/
   public void refreshAccounts()
   {
      this.accounts = /*call to database to get list of accounts*/
   }

   //called by every request to web service
   public boolean isActiveAccount(String accountId)
   {
       Account a = map.get(accountId);
       return a == null ? false : a.isActive();
   }
}

I am concerned about thread safety. Does the Spring framework not handle cases where one request is reading from the list and it is currently being updated by another? I have used read/write locks before in other applications, but I have never thought about a case such as above before.

I was planning on using the bean as a singleton so I could reduce database load.

By the way, this is a follow up of the below question:

Java Memory Storage to Reduce Database Load - Safe?

EDIT:

So would code like this solve this problem:

/*called at creation, and then via API calls to inform service new users have 
         been added to the database by a separate program*/
       public void refreshAccounts()
       {
          //java.util.concurrent.locks.Lock
          final Lock w = lock.writeLock();
          w.lock();
          try{
               this.accounts = /*call to database to get list of accounts*/
          }
          finally{
             w.unlock();
          }
       }

       //called by every request to web service
       public boolean isActiveAccount(String accountId)
       {
           final Lock r = lock.readLock();
           r.lock();

           try{
               Account a = map.get(accountId);
           }
           finally{
               r.unlock();
           }
           return a == null ? false : a.isActive();
       }

Upvotes: 8

Views: 41624

Answers (4)

tgkprog
tgkprog

Reputation: 4598

we have many such meta data and have some 11 nodes running. on each app node we have static maps for such data so its one instance only, init from db at start up once in off peak hour every day or when support person triggers it. have an interal simple http post based API to send updates from 1 node to others for some of the data which we need updates in real time.

public AccountDao
{
   private static List<Account> accounts;
   private static List<String> activeAccounts;
   private NamedParameterJdbcTemplate njt;

   static {
       try{
        refreshAccounts();
       }catch(Exception e){
        //log but do not throw. any uncaught exceptions in static means your class is un-usable
       }
   }


   public AccountDao(Datasource ds)
   {
       this.njt = new NamedParameterJdbcTemplate(ds);
       //refreshAccounts();
   }

   /*called at creation, and then via API calls to inform service new users have 
     been added to the database by a separate program*/
   public void refreshAccounts()
   {
      this.accounts = /*call to database to get list of accounts*/
   }

   public void addAccount(Account acEditedOrAdded)
   {
      //add or reove from map onr row
      //can be called from this node or other node
      //meaning if you have 2 nodes, keep IP port of each or use a internal web service or the like to tell 
      //node B when a account id added or changed in node A ...
   }   

   //called by every request to web service
   public static boolean isActiveAccount(String accountId)
   {
       Account a = map.get(accountId);
       return a == null ? false : a.isActive();
   }
}

Upvotes: 0

Lan
Lan

Reputation: 6660

Spring framework does not do anything under the hood concerning the multithreaded behavior of a singleton bean. It is the developer's responsibility to deal with concurrency issue and thread safety of the singleton bean.

I would suggest reading the below article: Spring Singleton, Request, Session Beans and Thread Safety

Upvotes: 12

meriton
meriton

Reputation: 70564

You could have asked for clarification on my initial answer. Spring does not synchronize access to a bean. If you have a bean in the default scope (singleton), there will only be a single object for that bean, and all concurrent requests will access that object, requiring that object to the thread safe.

Most spring beans have no mutable state, and as such are trivially thread safe. Your bean has mutable state, so you need to ensure no thread sees a list of accounts the other thread is currently assembling.

The easiest way to do that is to make the accounts field volatile. That assumes that you assign the new list to the field after having filled it (as you appear to be doing).

private volatile List<Accounts> accounts;

Upvotes: 3

John Vint
John Vint

Reputation: 40256

As a singleton and non-synchronized, Spring will allow any number of threads to concurrently invoke isActiveAccount and refreshAccounts. So, no this class is not going to be thread-safe and will not reduce the database load.

Upvotes: 1

Related Questions