Doug Lea writes:

---------
In an exchange with Hans Boehm, he pointed out that contrary to
reasonable expectations users probably have, a WeakHashMap that
is not being modified is NOT concurrently readable by multiple threads.

As he said...


>> On the other hand, with two threads concurrently reading the WeakHashMap
>> (e.g. by calling size()), two threads end up concurrently removing
>> enqueued elements from the WeakHashMap at the same time, which I very
>> strongly suspect can result in a mangled data structure.
>>


The current wording of the javadoc spec might make you believe otherwise.
Rather than changing the spec, we should fix the code. It's easy --
just use a lock around the deletion code in expungeStaleEntries.
(And conveniently, the "queue" makes a perfectly good lock-object.
(The diffs below are mostly just indents.)

As I said to Hans...


>> It's a little weird
>> making this class only a little thread-safe, but you are right that as it
>> stands, someone will someday be unhappily surprised.


Could you file a bug report on this.

% diff 1.6.0/j2se/martin/j2se/src/share/classes/java/util/WeakHashMap.java
jsr166/src/main/java/util/
275,290c275,294
< int h = e.hash;
< int i = indexFor(h, table.length);
<
< Entry<K,V> prev = table[i];
< Entry<K,V> p = prev;
< while (p != null) {
< Entry<K,V> next = p.next;
< if (p == e) {
< if (prev == e)
< table[i] = next;
< else
< prev.next = next;
< e.next = null; // Help GC
< e.value = null; // " "
< size--;
< break;
---
 > synchronized(queue) {
 > int h = e.hash;
 > int i = indexFor(h, table.length);
 >
 > Entry<K,V> prev = table[i];
 > Entry<K,V> p = prev;
 > while (p != null) {
 > Entry<K,V> next = p.next;
 > if (p == e) {
 > if (prev == e)
 > table[i] = next;
 > else
 > prev.next = next;
 > e.next = null; // Help GC
 > e.value = null; // " "
 > size--;
 > break;
 > }
 > prev = p;
 > p = next;
292,293d295
< prev = p;
< p = next;
---------