cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ralph Goers <Ralph.Go...@dslextreme.com>
Subject Re: [IMP] synchronization on session object in Cocoon
Date Mon, 16 May 2005 04:39:28 GMT
Yes, there was considerable discussion about this.

Yes, the version in svn is wrong.  I was hoping it would get fixed after 
our discussion, but that hasn't happened, so I'll try to commit 
something tonight (it is still Sunday night here in California).

Ralph

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
>  
>


Mime
View raw message