JSF PhaseListener instances must be made thread-safe

I recently performed a Google Search for "phaselistener not thread safe" and realized that this issue is not widely published, so I'd like to take a moment to discuss it here.

Sooner or later, most JSF developers end up developing a PhaseListener implementation like BridgeDebugPhaseListener.java found in Liferay Faces Bridge. One thing to remember though, is that PhaseListener instances are application-wide Singletons that are referenced by the JSF Lifecycle, which itself is an application-wide Singleton. Since these instances are not created within the PortletRequest thread, they need to be made thread-safe.

First, I recommend reading an external blog post section titled Immutable Objects.

Second, I think easiest way for me to explain what is OK and what is NOT OK is by showing some example code:

	package foo;

import javax.faces.event.PhaseId;
import javax.faces.event.PhaseEvent;
import javax.faces.event.PhaseListener;

public class MyPhaseListener implements PhaseListener {

    // Class-level immutable (final) variables are thread-safe since they are read-only
    // and are initialized at about the same time as the constructor. The Java compiler
    // will also permit you to initialize final variables within the body of the constructor.
    private final String classLevelImmutableVariable1 = "cannot-change-me-1";
    private final String classLevelImmutableVariable2; // initialized in constructor

    // Class-level mutable (non-final) variables are NOT thread-safe. Don't use these
    // without synchronization!
    private String classLevelMutableVariable = "can-change-me-but-dont-do-it!";

    // Static final constants (like loggers) are typically thread-safe.
    private static final Logger logger = LoggerFactory.getLogger(MyPhaseListener.class);

    /**
     * PhaseListener constructors execute when Mojarra/MyFaces initializes.
     */
    public MyPhaseListener() {
        // It's OK to initialize final class-level variables here.
        this.classLevelImmutableVariable2 = "cannot-change-me-2";
    }

    public void afterPhase(PhaseEvent phaseEvent) {

        // Accessing method parameters is thread-safe.
        PhaseId phaseId = phaseEvent.getPhaseId();

        // Accessing FacesContext is thread-safe since it is a ThreadLocal singleton.
        FacesContext facesContext = phaseEvent.getFacesContext();
    }

    public void beforePhase(PhaseEvent phaseEvent) {

        // Creating local method variables is thread-safe, since the JVM will create an
        // instance (copy) on the stack for each Thread.
        String mutableLocalMethodVariable = "ok-to-change-me-in-the-scope-of-this-method";

        // Writing to a logger is thread-safe, providing the underlying logger
        // implementation is thread-safe!
        logger.debug("inside beforePhase method");
    }
}

Finally, the bottom line is that the afterPhase(PhaseEvent) and beforePhase(PhaseEvent) methods must not modify class-level mutable (non-final) variables, or any type of memory that is shared by other threads, unless the methods are synchronized or access to the shared data is synchronized. Having said that, relying on synchronization can sometimes cause bottlenecks in an application because it causes other threads to block.