Session safety violation in Roller

Question

RollerSession has the following code:

public static RollerSession getRollerSession(HttpServletRequest request) {
    RollerSession rollerSession = null;
    HttpSession session = request.getSession(false);
    if (session != null) {
        rollerSession = (RollerSession)session.getAttribute(ROLLER_SESSION);
        if (rollerSession == null) {
            // HttpSession with no RollerSession?
            // Must be a session that was de-serialized from a previous run.
            rollerSession = new RollerSession();
            session.setAttribute(ROLLER_SESSION, rollerSession);
        }
      ....

I am new to concurrency problems. Here there seems to be an atomicity violation with two different threads potentially updating setAttribute concurrently. Is that right? Can session be shared by two threads given that it is being obtained from request?

Answer 1

Yes, you are right, more over there is a visibility problem too! According to IBM's post and Java Ranch the get/set operation are NOT thread-safe. So if you don't want any race conditions in your app, you should synchronize, but be careful where you put the synchronized.

EXPLANATION

Multiple servlets executing request threads may have active access to the same session object at the same time. The container must ensure that manipulation of internal data structures representing the session attributes is performed in a threadsafe manner. The Developer has the responsibility for threadsafe access to the attribute objects themselves. This will protect the attribute collection inside the HttpSession object from concurrent access, eliminating the opportunity for an application to cause that collection to become corrupted.

This is safe:

// guaranteed by the spec to be safe
request.getSession().setAttribute("foo", 1);

This is NOT safe:

HttpSession session = request.getSession();
Integer n = (Integer) session.getAttribute("foo");
// not thread safe
// another thread might be have got stale value between get and set
session.setAttribute("foo", (n == null) ? 1 : n + 1);

-- McDowell's Answer