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; ---------
|