Using request.getSession() as a locking object?

Question

I have some java code that gets and sets a session attribute:

Object obj = session.getAttribute(TEST_ATTR);
if (obj==null) {
  obj = new MyObject();
  session.setAttribute(obj);
}

In order to make this code thread-safe, I'd like to wrap it in a synchronized block. But what do I use as the locking object? Does it make sense to use the session?

synchronized (session) {
  Object obj = session.getAttribute(TEST_ATTR);
  if (obj==null) {
    obj = new MyObject();
    session.setAttribute(obj);
  }
}

Answer 1

It is generally frowned upon to use a lock that you have no control over. A lock should be scoped as tightly as possible and since the session is more or less a global object, it does not fit the bill. Try to use a separate lock from the java.util.concurrent.locks package and scope it to your class.

Answer 2

In the context of servlets? Servlets can be distributed across multiple processes, therefore you can't always have the same session object. A corollary to this is that a servlet container may decide to give you a different session object in the same process.

IIRC, Brian Goetz wrote an interesting article on the difficulty of doing things right with sessions.

My advice: Stay clear of sessions as much as possible, and don't lock random objects (use a lock object that has no other purpose).

Answer 3

I took a look at the article you posted. You could skip synchronizing all together and take the same approach that the author did by using compare-and-set to ensure that your data is correct:

ServletContext ctx = getServletConfig().getServletContext();
AtomicReference<TYPE> holder 
    = (AtomicReference<TYPE>) ctx.getAttribute(TEST_ATTR);
while (true) {
    TYPE oldVal = holder.get();
    TYPE newVal = computeNewVal(oldVal);
    if (holder.compareAndSet(oldVal, newVal))
        break;
} 

holder.compareAndSet(old, new) will return false if some other thread updated the value of "holder" since you last read it. holder.compareAndSet(,) is put in a while(true) loop so that if the value did change before you were able to write it then you get a chance to read the value again and re-try your write.

http://java.sun.com/javase/6/docs/api/java/util/concurrent/atomic/AtomicReference.html