Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Difference between volatile Boolean and Boolean

Suppose I declare like this:

 private static  Boolean isCondition = false;

Then I am using this like below in synchronized statement:

synchronized(isCondition){
               isCondition = true;
               somestuff();
             }

Here my question is if I update isCondition then it will get a new reference due to autoboxing and if new thread will come in synchronized block then they will get lock on new object enter into synchronized block. This I dont want to happen.

So please suggest me alternatives and if I use volatile then how exactly it will prevent this like below:

private static volatile Boolean isCondition = false;

The actual code is like that:

package com.test.spring.utils;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.beans.BeansException;
import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
import org.springframework.context.support.ClassPathXmlApplicationContext;

 /**
  * @author Pratik
*/
public class TouchPointsSpringContext implements ApplicationContextAware
 {
 private static final Log g_log = LogFactory.getLog(TouchPointsSpringContext.class);
 private static ApplicationContext CONTEXT;
 private static volatile Boolean isServiceInitialized = false;

/**
 * This method is called from within the ApplicationContext once it is done
 * starting up, it will stick a reference to itself into this bean.
 *  
 * @param context
 *            a reference to the ApplicationContext.
 */
public void setApplicationContext(ApplicationContext context) throws BeansException
{
    CONTEXT = context;
}

private static void initializeTouchPointService()
{
    g_log.info("getting touchpoints service application context");
    String[] locations =
            { "appContext-main.xml", "appContext-hibernate.xml" };
    ApplicationContext applicationContext = new ClassPathXmlApplicationContext(locations);
    g_log.info("setting touchpoints service application context");
    CONTEXT = applicationContext;
}
 /**
 * This is about the same as context.getBean("beanName"), except it has its
 * own static handle to the Spring context, so calling this method
 * statically will give access to the beans by name in the Spring
 * application context. As in the context.getBean("beanName") call, the
 * caller must cast to the appropriate target class. If the bean does not
 * exist, then a Runtime error will be thrown.
 * 
 * @param beanName
 *            the name of the bean to get.
 * @return an Object reference to the named bean.
 */
public static Object getBean(String beanName)
{
    if (!isServiceInitialized || (CONTEXT == null))
    {
        synchronized (isServiceInitialized)
        {
            if (!isServiceInitialized)
            {
                initializeTouchPointService();
                isServiceInitialized = true;
            }
        }
    }
    return CONTEXT.getBean(beanName);
}

public static void main(String[] args)
{
    TouchPointsSpringContext.getBean("lookupService");
}

}

like image 277
Pratik Suman Avatar asked Jul 02 '14 12:07

Pratik Suman


2 Answers

Using a Boolean as a lock is a very bad idea: you are effectively using a global variable Boolean.TRUE/FALSE which any other parts of your code can access and potentially deadlock your code.

And using a non final variable as a lock is an even worse idea: everytime you reallocate the instance (isCondition = true) you change your lock, meaning that two threads may execute your synchronized block concurrently, which kind of defeats the whole idea.

So I would recommend a standard idiom:

private static final Object lock = new Object();
private static boolean isCondition;

synchronised(lock) {
    isCondition = true;
    // ...
}
like image 154
assylias Avatar answered Sep 28 '22 23:09

assylias


I think most of the other answers here are not completely right. It is a little bit difficult to understand what you are doing because you do not include the code for initializeTouchPointService, however you appear to be doing something which is a variation on the "Double Checked Locking" idiom.

It is difficult to get this concurrency idiom right and if you are using a version of Java before 5, then you should not attempt to use this idiom at all. I will assume you are using Java 5+.

The important parts of your code are:

private static ApplicationContext CONTEXT;
private static volatile Boolean isServiceInitialized = false;

...

if (!isServiceInitialized || (CONTEXT == null))
{
    synchronized (isServiceInitialized)
    {
        if (!isServiceInitialized)
        {
            initializeTouchPointService();
            isServiceInitialized = true;
        }
    }
}

Assuming that you are using Java 5 or above, you must use volatile on all relevant variables to get this idiom to work correctly. You must also re-check the full condition inside the synchronized block.

You must not use a Boolean as your lock, since Boolean objects are immutable you will get a different object when you change the condition from false to true. Instead use a separate lock object and boolean primitive for the condition.

    private final Object lock = new Object();
    private volatile boolean isServiceInitialized;
    private volatile ApplicationContext context; 

    public Object getBean(String beanName) {
        if (!isServiceInitialized || context == null) {
            synchronized(lock) {
                if (!isServiceInitialized || context == null) {
                    initializeTouchPointService();
                    isServiceInitialized = true;
               }
            }
        }
        return CONTEXT.getBean(beanName);
    }

However, locks in recent versions of Java have very good performance on most architectures. So using the double-checked locking idiom may not make your program much faster - especially compared to how slow spring reflection will be when calling getBean.

Instead of your double-checked design, how about the following simpler design which also avoids volatile:

   private final Object lock = new Object();
   private boolean isServiceInitialized;
   private ApplicationContext context; 

   private ApplicationContext context() {
       synchronized(lock) {
            if (!isServiceInitialized || context == null) {
                initializeTouchPointService();
                condition = true;
            }
            return context;
        }
   }

   public Object getBean(String beanName) {
        return context().getBean(beanName);
   }

I also recommend avoiding the use of static where ever possible, as writing unit tests in the presence of global variables can be tricky. I would seriously consider if there is any way you can change your design to reduce or eliminate your use of static state.

============ edit

Based on my best guess of what the OP is trying to achieve, perhaps this would be better. However, it removes the lazy initialisation. So if you program sometimes refers to this TouchPointsSpringContext class without using the getBean() method then you don't want this answer.

public class TouchPointsSpringContext
{
  private static final Log g_log = LogFactory.getLog(TouchPointsSpringContext.class);
  private static ApplicationContext CONTEXT = initializeTouchPointService();

  private static ApplicationContext initializeTouchPointService()
  {
      g_log.info("getting touchpoints service application context");
      String[] locations =
        { "appContext-main.xml", "appContext-hibernate.xml" };
      ApplicationContext applicationContext = new ClassPathXmlApplicationContext(locations);
      g_log.info("setting touchpoints service application context");
      return applicationContext;
  }

  public static Object getBean(String beanName) 
  {
      return CONTEXT.getBean(beanName);
  }

  public static void main(String[] args)
  {
      TouchPointsSpringContext.getBean("lookupService");
  }
}

Note that the JVM will automatically make sure that your static CONTEXT gets initalised exactly once.

Or alternatively, if you can avoid implementing "ApplicationContextAware" (implementing it seems unnecessary given the rest of the code), but you need to keep he lazy initialisation, then this might be better:

public class TouchPointsSpringContext
{
  private static final Log g_log = LogFactory.getLog(TouchPointsSpringContext.class);
  private static volatile ApplicationContext CONTEXT;
  private static final Object lock = new Object();

  private static ApplicationContext initializeTouchPointService()
  {
    g_log.info("getting touchpoints service application context");
    String[] locations =
        { "appContext-main.xml", "appContext-hibernate.xml" };
    ApplicationContext applicationContext = new ClassPathXmlApplicationContext(locations);
    g_log.info("setting touchpoints service application context");
    return applicationContext;
  }

  public static Object getBean(String beanName)
  {
    if (CONTEXT == null)
    {
        synchronized (lock)
        {
           if (CONTEXT == null)
           {
              CONTEXT = initializeTouchPointService();
           }
        }
    }
    return CONTEXT.getBean(beanName);
   }

   public static void main(String[] args)
   {
       TouchPointsSpringContext.getBean("lookupService");
   }
}
like image 34
lexicalscope Avatar answered Sep 28 '22 23:09

lexicalscope