Am I not getting it, or is the implementation broken (see below)?
/** The map to assure 1:1-mapping of server sessions and Cocoon session
wrappers */
private static final Map sessions = Collections.synchronizedMap(new
WeakHashMap());
public Session getSession(boolean create) {
javax.servlet.http.HttpSession serverSession =
this.req.getSession(create);
HttpSession session;
if (serverSession != null) {
// no need to lock the map - it is synchronized
// synch on server session assures only one wrapper per
session
synchronized (serverSession) {
// retrieve existing wrapper
session =
(HttpSession)((WeakReference)sessions.get(serverSession)).get();
//----> What if the map does not contain the session?
sessions.get(serverSessions) returns null, resulting in a
NullPointerException in the second get().
//----> Why not synchronize on the map, and not the serverSession. Right
now, we run through 3 synchronize blocks: synchronized(serverSession),
session.get(...) and sessions.put(...). Wouldn't replacing
synchronized(serverSession) with synchronized(sessions) and using an
unsynchronized map suffice? Resulting in only one synchronized block?
if (session == null) {
// create new wrapper
session = new HttpSession(serverSession);
sessions.put(serverSession, new
WeakReference(session));
}
}
} else {
// invalidate
session = null;
}
return session;
}
Regards,
Jochen
