Please update from svn and review the changes.
Ralhp
Jochen Kuhnle wrote:
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
