DO NOT REPLY [Bug 36541] New: - session getAttribute/setAttribute and removeAttribute are NOT Thread safe.

[bu...@apache.org]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=36541>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=36541

           Summary: session getAttribute/setAttribute and removeAttribute
                    are NOT Thread safe.
           Product: Tomcat 5
           Version: 5.0.25
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: critical
          Priority: P1
         Component: Catalina
        AssignedTo: tomcat-dev@jakarta.apache.org
        ReportedBy: struts_user@anotheria.net


I'm not quite sure if it's a bug or spec flaw, but I talked to Craig McClanahan
and he encouraged me to submit it.
The session attribute handling methods in 5.0.x aren't thread safe. The
org.apache.catalina.session.StandardSession and StadardSessionFacade do not
synchronize in get/set/remove Attribute. The result is following:
If you write and read from the session simultaneously with multiple threads the
getter/setter methods corrupt the underlying HashMap. The HashMap's entries got
circularly linked, and any thread using a get() on such a HashMap will spin
forever chasing its tail (quoted from Larry Isaacs). 

Now what Josh Bloch and Co. are saying in the javadoc for HashMap:
 * <p><b>Note that this implementation is not synchronized.</b> If multiple
 * threads access this map concurrently, and at least one of the threads
 * modifies the map structurally, it <i>must</i> be synchronized externally.
 * (A structural modification is any operation that adds or deletes one or
 * more mappings; merely changing the value associated with a key that an
 * instance already contains is not a structural modification.)  This is
 * typically accomplished by synchronizing on some object that naturally
 * encapsulates the map.  If no such object exists, the map should be
 * "wrapped" using the <tt>Collections.synchronizedMap</tt> method.  This is
 * best done at creation time, to prevent accidental unsynchronized access to
 * the map: <pre> Map m = Collections.synchronizedMap(new HashMap(...));
 * </pre>

The bug is quite easy to fix, by making the map synchronized (as stated above)
or explicitely synchronize the places where HashMap.get() or put() is called. 
I could provide a patch if wished.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org