Index: lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java =================================================================== --- lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java (revision 1352711) +++ lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java (working copy) @@ -27,9 +27,7 @@ import java.nio.channels.FileChannel; import java.nio.channels.FileChannel.MapMode; -import java.util.Collections; -import java.util.Set; -import java.util.WeakHashMap; +import java.util.Iterator; import java.security.AccessController; import java.security.PrivilegedExceptionAction; @@ -37,6 +35,7 @@ import java.lang.reflect.Method; import org.apache.lucene.util.Constants; +import org.apache.lucene.util.WeakIdentityMap; /** File-based {@link Directory} implementation that uses * mmap for reading, and {@link @@ -261,7 +260,7 @@ private ByteBuffer curBuf; // redundant for speed: buffers[curBufIndex] private boolean isClone = false; - private final Set clones = Collections.newSetFromMap(new WeakHashMap()); + private final WeakIdentityMap clones = WeakIdentityMap.newConcurrentHashMap(); MMapIndexInput(String resourceDescription, RandomAccessFile raf, long offset, long length, int chunkSizePower) throws IOException { super(resourceDescription); @@ -431,9 +430,7 @@ } // register the new clone in our clone list to clean it up on closing: - synchronized(this.clones) { - this.clones.add(clone); - } + this.clones.put(clone, Boolean.TRUE); return clone; } @@ -449,35 +446,25 @@ try { if (isClone || buffers == null) return; + // make local copy, then un-set early + final ByteBuffer[] bufs = buffers; + unsetBuffers(); + // for extra safety unset also all clones' buffers: - synchronized(this.clones) { - for (final MMapIndexInput clone : this.clones) { - assert clone.isClone; - clone.unsetBuffers(); - } - this.clones.clear(); + for (Iterator it = this.clones.keyIterator(); it.hasNext();) { + final MMapIndexInput clone = it.next(); + assert clone.isClone; + clone.unsetBuffers(); } + this.clones.clear(); - curBuf = null; curBufIndex = 0; // nuke curr pointer early - for (int bufNr = 0; bufNr < buffers.length; bufNr++) { - cleanMapping(buffers[bufNr]); + for (final ByteBuffer b : bufs) { + cleanMapping(b); } } finally { unsetBuffers(); } } - - // make sure we have identity on equals/hashCode for WeakHashMap - @Override - public int hashCode() { - return System.identityHashCode(this); - } - - // make sure we have identity on equals/hashCode for WeakHashMap - @Override - public boolean equals(Object obj) { - return obj == this; - } } } Index: lucene/core/src/java/org/apache/lucene/util/WeakIdentityMap.java =================================================================== --- lucene/core/src/java/org/apache/lucene/util/WeakIdentityMap.java (revision 1352711) +++ lucene/core/src/java/org/apache/lucene/util/WeakIdentityMap.java (working copy) @@ -21,7 +21,9 @@ import java.lang.ref.ReferenceQueue; import java.lang.ref.WeakReference; import java.util.HashMap; +import java.util.Iterator; import java.util.Map; +import java.util.NoSuchElementException; import java.util.concurrent.ConcurrentHashMap; /** @@ -97,6 +99,65 @@ reap(); return backingStore.size(); } + + /** Returns an iterator over all weak keys of this map. + * Keys already garbage collected will not be returned. + * This Iterator does not support removals. */ + public Iterator keyIterator() { + reap(); + final Iterator iterator = backingStore.keySet().iterator(); + return new Iterator() { + // holds strong reference to next element in backing iterator: + private Object next = null; + // the backing iterator was already consumed: + private boolean nextIsSet = false; + + @Override + public boolean hasNext() { + return nextIsSet ? true : setNext(); + } + + @Override @SuppressWarnings("unchecked") + public K next() { + if (nextIsSet || setNext()) { + assert nextIsSet; + nextIsSet = false; // invalidate current value! + return (K) next; + } + throw new NoSuchElementException(); + } + + @Override + public void remove() { + throw new UnsupportedOperationException(); + } + + private boolean setNext() { + assert !nextIsSet; + while (iterator.hasNext()) { + next = iterator.next().get(); + if (next == null) { + // already garbage collected! + continue; + } + // unfold "null" special value + if (next == NULL) { + next = null; + } + return nextIsSet = true; + } + return false; + } + }; + } + + /** Returns an iterator over all values of this map. + * This iterator may return values whose key is already + * garbage collected while iterator is consumed. */ + public Iterator valueIterator() { + reap(); + return backingStore.values().iterator(); + } private void reap() { Reference zombie; @@ -104,6 +165,9 @@ backingStore.remove(zombie); } } + + // we keep a hard reference to our NULL key, so map supports null keys that never get GCed: + static final Object NULL = new Object(); private static final class IdentityWeakReference extends WeakReference { private final int hash; @@ -129,9 +193,6 @@ } return false; } - - // we keep a hard reference to our NULL key, so map supports null keys that never get GCed: - private static final Object NULL = new Object(); } } Index: lucene/core/src/test/org/apache/lucene/util/TestWeakIdentityMap.java =================================================================== --- lucene/core/src/test/org/apache/lucene/util/TestWeakIdentityMap.java (revision 1352711) +++ lucene/core/src/test/org/apache/lucene/util/TestWeakIdentityMap.java (working copy) @@ -17,7 +17,9 @@ package org.apache.lucene.util; +import java.util.Iterator; import java.util.Map; +import java.util.NoSuchElementException; import java.util.Random; import java.util.concurrent.atomic.AtomicReferenceArray; import java.util.concurrent.Executors; @@ -84,6 +86,22 @@ map.put(key3, "bar3"); assertEquals(3, map.size()); + int c = 0; + for (Iterator it = map.keyIterator(); it.hasNext();) { + final String k = it.next(); + assertTrue(k == key1 || k == key2 | k == key3); + c++; + } + assertEquals(3, c); + + c = 0; + for (Iterator it = map.valueIterator(); it.hasNext();) { + final String v = it.next(); + assertTrue(v.startsWith("bar")); + c++; + } + assertEquals(3, c); + // clear strong refs key1 = key2 = key3 = null; @@ -93,7 +111,13 @@ System.runFinalization(); System.gc(); Thread.sleep(100L); - assertTrue(size >= map.size()); + c = 0; + for (Iterator it = map.keyIterator(); it.hasNext();) { + assertNotNull(it.next()); + c++; + } + assertTrue(size >= c); + assertTrue(c >= map.size()); size = map.size(); } catch (InterruptedException ie) {} @@ -101,6 +125,14 @@ assertEquals(0, map.size()); assertTrue(map.isEmpty()); + Iterator it = map.keyIterator(); + assertFalse(it.hasNext()); + try { + it.next(); + fail("Should throw NoSuchElementException"); + } catch (NoSuchElementException nse) { + } + key1 = new String("foo"); key2 = new String("foo"); map.put(key1, "bar1"); @@ -133,7 +165,7 @@ final int count = atLeast(rnd, 10000); for (int i = 0; i < count; i++) { final int j = rnd.nextInt(keyCount); - switch (rnd.nextInt(4)) { + switch (rnd.nextInt(5)) { case 0: map.put(keys.get(j), Integer.valueOf(j)); break; @@ -150,6 +182,12 @@ // renew key, the old one will be GCed at some time: keys.set(j, new Object()); break; + case 4: + // check iterator still working + for (Iterator it = map.keyIterator(); it.hasNext();) { + assertNotNull(it.next()); + } + break; default: fail("Should not get here."); } @@ -173,7 +211,13 @@ System.runFinalization(); System.gc(); Thread.sleep(100L); - assertTrue(size >= map.size()); + int c = 0; + for (Iterator it = map.keyIterator(); it.hasNext();) { + assertNotNull(it.next()); + c++; + } + assertTrue(size >= c); + assertTrue(c >= map.size()); size = map.size(); } catch (InterruptedException ie) {} }