Is the following utility class thread-safe?

Question

First let's look at the utility class (most javadoc has been removed to simply the example):

public class ApplicationContextUtils {

    /**
     * The application context; care should be taken to ensure that 1) this
     * variable is assigned exactly once (in the
     * {@link #setContext(ApplicationContext)} method, 2) the context is never
     * reassigned to {@code null}, 3) access to the field is thread-safe (no race
     * conditions can occur)
     */
    private static ApplicationContext context = null;

    public static ApplicationContext getContext() {

    if (!isInitialized()) {
        throw new IllegalStateException(
            "Context not initialized yet! (Has the "
                + "ApplicationContextProviderBean definition been configured "
                + "properly and has the web application finished "
                + "loading before you invoked this method?)");
    }

    return context;
    }

    public static boolean isInitialized() {
    return context == null;
    }

    @SuppressWarnings("unchecked")
    public static <T> T getBean(final String name, final Class<T> requiredType) {
    if (requiredType == null) {
        throw new IllegalArgumentException("requiredType is null");
    }
    return (T) getContext().getBean(name, requiredType);
    }

    static synchronized void setContext(final ApplicationContext theContext) {

    if (theContext == null) {
        throw new IllegalArgumentException("theContext is null");
    }

    if (context != null) {
        throw new IllegalStateException(
            "ApplicationContext already initialized: it cannot be done twice!");
    }

    context = theContext;
    }

    private ApplicationContextUtils() {
    throw new AssertionError(); // NON-INSTANTIABLE UTILITY CLASS
    }
}

Finally, there is the following helper Spring managed bean that actually calls the 'setContext' method:

public final class ApplicationContextProviderBean implements
    ApplicationContextAware {

    public void setApplicationContext(
        final ApplicationContext applicationContext) throws BeansException {
    ApplicationContextUtils.setContext(applicationContext);
    }
}

Spring will call the setApplicationContext method once after the app is started. Assuming a nincompoop has not previously called ApplicationContextUtils.setContext(), that should lock in the reference to the context in the utility class, allowing calls to getContext() to success (meaning that isInitialized() returns true).

I just want to know if this class violates any principles of good coding practices, with respect to thread safety in particular (but other stupidities found are welcome).

Thanks for helping me to become a better programmer, StackOverflow!

Regards, LES

P.S. I didn't go into why I need this utility class - let it suffice that I indeed do have a legitimate need to access it from a static context anywhere in the application (after the spring context has loaded, of course).

Answer 1

No. It's not thread safe.

Writes to the context class variable are not guaranteed to be visible to threads that read that variable through getContext().

At the very least, declare context to be volatile. Ideally, redefine context as an AtomicReference, set through a call like this:

if(!context.compareAndSet(null, theContext))
  throw new IllegalStateException("The context is already set.");

---

Here's a more complete example:

public class ApplicationContextUtils {

  private static final AtomicReference<ApplicationContext> context = 
    new AtomicReference<ApplicationContext>();

  public static ApplicationContext getContext() {
    ApplicationContext ctx = context.get();
    if (ctx == null)
      throw new IllegalStateException();
    return ctx;
  }

  public static boolean isInitialized() {
    return context.get() == null;
  }

  static void setContext(final ApplicationContext ctx) {
    if (ctx == null) 
      throw new IllegalArgumentException();
    if (!context.compareAndSet(null, ctx))
      throw new IllegalStateException();
  }

  public static <T> T getBean(final String name, final Class<T> type) {
    if (type == null) 
      throw new IllegalArgumentException();
    return type.cast(getContext().getBean(name, type));
  }

  private ApplicationContextUtils() {
    throw new AssertionError();
  }

}

Note that in addition to thread safety, this also provides type safety, taking advantage of the Class instance passed into the getBean() method.

I'm not sure how you plan to use the isInitialized() method; it doesn't seem very useful to me, since as soon as you call it, the condition could change, and you don't have a good way to be notified.