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

Answer 4

The spec does not guarantee that this will help at all:

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

(It might work for specific implementations, but there are no guarantees that it will work in all containers.)

Servlet 2.5 MR6 says:

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.

Basically, the spec makes it your problem. Your solution will have to be tailored to your application design and deployment plan. I'm not sure that there is a global solution to the problem that will work in all cases; however, you may get a better answer if you are more specific about the architecture of your application and configuration of your application server.

Answer 5

Your code won't work for at least two reasons.

1) If the session doesn't exist, then you could easily create it twice for the same user, and have an ugly race condition.

2) If the session is not the same object across threads, then it won't work anyway. The session will probably be equals() to the same session in another thread, but that won't work.

Answer 6

You don't need to lock since session.setAttribute() is thread safe (see servlet spec comment from @McDowell above).

However, let's use a different example. Let's say you wanted to chek the value of the attribute, then update it if <= 100. In this case you would need to syncronize the block of code for the getAttribute() the compare <= 100 and the setAttribute().

Now, what should you use for the lock? Remember there is no syncronization if different objects are used for the lock. So different code blocks must use the same object. Your choice of the session object may be just fine. Remember too that different code blocks may access the session (both read/write) even if you have taken a lock, unless that other code also locks on the session object. A pitfall here is that too many places in your code take a lock on the session object and thus have to wait. For example, if your code block uses session attribute A and another hunk of code uses session attribute B, it would be nice if they didn't need to wait on each other by both taking a lock on the session object. The use of static objects named LockForA, and LockForB might be better choices for your code to use - e.g. synchronized (LockForA) { }.

Answer 7

I had the same problem and didn't want to scope it to my class because the creation of my object might take a second and stall threads of others sessions where the object was created already. I didn't want to use the session object of the request to synchronize on because the application server implementation might return a differen session facade in different requests and synchronizing on different objects simply doesn't work. So I choose to put an object on the session to use as a lock and use the double check idiom to ensure the lock is created only once. Synchronizing in the scope of MyObject is not a problem any more because creating an object is quite fast.

Object lock = session.getAttribute("SessionLock");
if (lock == null) {
  synchronized (MyObject.class) {
    lock = session.getAttribute("SessionLock");
    if(lock == null) {
      lock = new Object();
      session.setAttribute("SessionLock", lock);
    }
  }
}
synchronized (lock) {
  Object obj = session.getAttribute(TEST_ATTR);
  if (obj==null) {
    obj = new MyObject();
    session.setAttribute(obj);
  }
}