views:

126

answers:

3

I have servlets that caches user information rather then retrieving it from the user store on every request (shared Ehcache). The issue I have is that if a client is multi-threaded and they make more then one simultaneous request, before they have been authenticated, then I get this in my log:

Retrieving User [Bob]
Retrieving User [Bob]
Retrieving User [Bob]
Returned [Bob] ...caching
Returned [Bob] ...caching
Returned [Bob] ...caching 

What I would want is that the first request would call the user service, while the other two requests get blocked - and when the first request returns, and then caches the object, the other two requests go through:

Retrieving User [Bob]
blocking...
blocking...
Returned [Bob] ...caching
[Bob] found in cache
[Bob] found in cache

I've thought about locking on the String "Bob" (because due to interning it's always the same object right?). Would that work? And if so how do I keep track of the keys that actually exist in the cache and build a locking mechanism around them that would then return the valid object once it's retrieved. Thanks.

A: 

If you are caching with a Map, then locking on the key would do what you suggest.

CDSO1
+4  A: 

You can't guarantee that there will be no deadlock if you don't have exclusive control of your locks. Interned String are globally visible, so they are a poor candidate.

Instead, use a map between keys and their corresponding locks. You might use synchronized access to a concurrent map, or use a ConcurrentMap. I'm not sure which is cheaper, but I'd lean toward ConcurrentMap because it expresses your intent succinctly.

ReadWriteLock trial = new ReentrantReadWriteLock(fair);
ReadWriteLock lock = locks.putIfAbsent(key, trial);
if (lock == null) {
  /* The current thread won the race to create lock for key. */
  lock = trial;
}

(Using a ReadWriteLock is optional; with it, you could do something fancy like allowing concurrent reads of a cached value by multiple threads, yet still have another thread get an exclusive lock when the value needs to be updated.)

It might be expensive to create a lot of locks that end up being discarded because a lock already exists. Or, you might use an old runtime without java.util.concurrent. In cases like those you could synchronize on the map:

Object lock;
synchronized (locks) {
  if (locks.containsKey(key))
    lock = locks.get(key);
  else {
    lock = new Object();
    locks.put(key, object);
  }
}
erickson
This is pretty close to what I was thinking, locking on the interned Strings just seemed like a really bad idea for some reason. But in this scenario I am locking every call on the Map - but I guess this isn't that different (performance wise) then my cache which is shared by all the threads. I would think even this overhead is still way faster then making multiple unneeded calls to my user authentication provider.
Gandalf
+2  A: 

I've thought about locking on the String "Bob" (because due to interning it's always the same object right?). Would that work?

I've previously tried this and it actually doesn't work quite like you might expect. You need to make sure you call intern() on the Strings first; however, synchronizing on intern strings is actually a really bad idea.

matt b