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

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 3

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 4

The synchronization of session access makes no sense. It would only be useful if the same user is invoking two requests at the exact same time. But this wouldn't occur in real world and even if it would, there's means of some kind of hacker and it really isn't worth the effort.

If it were a ServletContext attribute or global/static webapp variable (e.g. count visits) which can be accessed by multiple users (from different sessions), then synchronizing it would maybe make a bit more sense, because from the thousands users accessing it, the chance is a bit bigger that the variable is accessed at the exact same time (we're still talking about time ranges in micro/nanoseconds).

After all, in real you don't need to worry about this. Your first way is perfectly valid.

Answer 5

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 6

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.