In the bug report 3884, Craig wrote...
> However, instead
> of synchronizing on the entire session object (which is similar to what STM
> does
> on the entire servlet), the internal code locks *only* on the attributes
> collection:
>
> public void setAttribute(String name, Object value) {
> ...
> synchronized (attributes) {
> ...
> attributes.put(name, value);
> ...
> }
> ...
> }
>
> so that no other calls to the session that are happening at the same time from
> other threads (like getId() or setMaxInactiveInterval()) are affected by these
> locks.
The attributes 'collection' is a HashMap.
The JDK 1.2 javadocs for HashMap write:
Note that this implementation is not synchronized. If multiple threads
access this map concurrently, and at least one of the threads modifies the
map structurally, it must 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.
The last sentence there is the important one that I care about.
Shouldn't it be:
synchronized (this)
????
-jon
