Unsynchronized methodCache in DefaultRemoter

[tpeierls]

In 2.0.rc3, DefaultRemoter.methodCache is a HashMap that is potentially accessed by multiple threads without synchronization. HashMap is not thread-safe, so this code needs to be fixed. (Java Concurrency in Practice, p.16)

Simplest fix is to wrap the initialization with Collections.synchronizedMap.

The comment about performance (line 205) should be tweaked. Even with synchronizedMap(), getMethodJS() can be called more than strictly necessary due to non-atomic check-then-set, but it's still not a big issue. (If you really wanted to plug that gap, you could use the Memoizer pattern, JCiP p.108, but I think that would be serious overkill.)

--tim

[tpeierls]

I entered this in the bug tracker.

tpeierls wrote:

In 2.0.rc3, DefaultRemoter.methodCache is a HashMap that is potentially accessed by multiple threads without synchronization. HashMap is not thread-safe, so this code needs to be fixed. (Java Concurrency in Practice, p.16)

Simplest fix is to wrap the initialization with Collections.synchronizedMap.

The comment about performance (line 205) should be tweaked. Even with synchronizedMap(), getMethodJS() can be called more than strictly necessary due to non-atomic check-then-set, but it's still not a big issue. (If you really wanted to plug that gap, you could use the Memoizer pattern, JCiP p.108, but I think that would be serious overkill.)

--tim