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: svn commit: r169856 - /cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/http/HttpRequest.java
Date Fri, 13 May 2005 06:57:14 GMT
Nathaniel Alfred wrote:

>One must synchonize the put and get operation on the map itself
>in order to protect its internal consistency.
>  
>
Well, yeah.  A synchronized block that synchronizes on the map 
accomplishes that.

>  Map map = Collections.synchronizedMap(new HashMap());
>  ...
>  map.put(key, value);
>  ...
>  value = map.get(key);
>
>is just easier to read than
>
>  Map map = new HashMap();
>  ...
>  synchronized(map) { map.put(key, value); }
>  ...
>  synchronized(map) { value = map.get(key); }
>  
>
Except that in this case it is:

            synchronized (sessions) {
                // retrieve existing wrapper
                session = (HttpSession)((WeakReference)sessions.get(serverSession)).get();
                if (session == null) {
                    // create new wrapper
                    session = new HttpSession(serverSession);
                    sessions.put(serverSession, new WeakReference(session));
                }
            }

>although with just one get and one put the significance may be argued.
>
>You don't want to replace the outer synchronized(serverSession) by 
>synchronized(sessions) because that blocks all threads without being
>necessary (although the effect will be immeasurable).
>  
>
Yes, I do. The alternative is to synchronize on the servlet container's 
session object, which could have far more horrifying results. Do you 
have any idea how that will impact the container?  I don't because I 
cannot know.  For all I know it is conceivable that doing that could 
cause a deadlock in some wierd scenario.  I also fail to see how locking 
the map causes any more of a performance hit than locking the session. 
Since the map is only used by this one method its effect should be far 
less of an impact that locking the session.  Besides, you ARE blocking 
all threads on the get and put anyway, since it has been declared a 
synchronized hash map. In fact, it is being done twice inside of a 
synchronized block.

>You must have synchronized(serverSession) (or block all threads) to 
>avoid calling sessions.put twice for the same serverSession.
>  
>
I don't see two calls to put in the code above....

>Cheers, Alfred.
>
>  
>
Ralph

Mime
View raw message