Index: lucene/src/java/org/apache/lucene/store/RAMDirectory.java =================================================================== --- lucene/src/java/org/apache/lucene/store/RAMDirectory.java (revision 1040423) +++ lucene/src/java/org/apache/lucene/store/RAMDirectory.java (working copy) @@ -20,8 +20,10 @@ import java.io.IOException; import java.io.FileNotFoundException; import java.io.Serializable; -import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; import org.apache.lucene.index.IndexFileNameFilter; @@ -36,7 +38,7 @@ private static final long serialVersionUID = 1l; - protected HashMap fileMap = new HashMap(); + protected final Map fileMap = new ConcurrentHashMap(); protected final AtomicLong sizeInBytes = new AtomicLong(); // ***** @@ -83,25 +85,20 @@ } @Override - public synchronized final String[] listAll() { + public final String[] listAll() { ensureOpen(); - Set fileNames = fileMap.keySet(); - String[] result = new String[fileNames.size()]; - int i = 0; - for(final String fileName: fileNames) - result[i++] = fileName; - return result; + // NOTE: we need to clone the key set to create a "point in time" snapshot + // of the map. Otherwise, if the map is changed during iteration, such as + // items are added or removed, exceptions could be thrown + // (ArrayIndexOutOfBoundException was actually thrown at some point). + return new HashSet(fileMap.keySet()).toArray(new String[0]); } /** Returns true iff the named file exists in this directory. */ @Override public final boolean fileExists(String name) { ensureOpen(); - RAMFile file; - synchronized (this) { - file = fileMap.get(name); - } - return file != null; + return fileMap.containsKey(name); } /** Returns the time the named file was last modified. @@ -110,12 +107,10 @@ @Override public final long fileModified(String name) throws IOException { ensureOpen(); - RAMFile file; - synchronized (this) { - file = fileMap.get(name); - } - if (file==null) + RAMFile file = fileMap.get(name); + if (file == null) { throw new FileNotFoundException(name); + } return file.getLastModified(); } @@ -125,12 +120,10 @@ @Override public void touchFile(String name) throws IOException { ensureOpen(); - RAMFile file; - synchronized (this) { - file = fileMap.get(name); - } - if (file==null) + RAMFile file = fileMap.get(name); + if (file == null) { throw new FileNotFoundException(name); + } long ts2, ts1 = System.currentTimeMillis(); do { @@ -151,19 +144,17 @@ @Override public final long fileLength(String name) throws IOException { ensureOpen(); - RAMFile file; - synchronized (this) { - file = fileMap.get(name); - } - if (file==null) + RAMFile file = fileMap.get(name); + if (file == null) { throw new FileNotFoundException(name); + } return file.getLength(); } /** Return total size in bytes of all files in this * directory. This is currently quantized to * RAMOutputStream.BUFFER_SIZE. */ - public synchronized final long sizeInBytes() { + public final long sizeInBytes() { ensureOpen(); return sizeInBytes.get(); } @@ -172,14 +163,15 @@ * @throws IOException if the file does not exist */ @Override - public synchronized void deleteFile(String name) throws IOException { + public void deleteFile(String name) throws IOException { ensureOpen(); RAMFile file = fileMap.remove(name); - if (file!=null) { + if (file != null) { file.directory = null; sizeInBytes.addAndGet(-file.sizeInBytes); - } else + } else { throw new FileNotFoundException(name); + } } /** Creates a new, empty file in the directory with the given name. Returns a stream writing this file. */ @@ -187,14 +179,12 @@ public IndexOutput createOutput(String name) throws IOException { ensureOpen(); RAMFile file = newRAMFile(); - synchronized (this) { - RAMFile existing = fileMap.get(name); - if (existing!=null) { - sizeInBytes.addAndGet(-existing.sizeInBytes); - existing.directory = null; - } - fileMap.put(name, file); + RAMFile existing = fileMap.remove(name); + if (existing != null) { + sizeInBytes.addAndGet(-existing.sizeInBytes); + existing.directory = null; } + fileMap.put(name, file); return new RAMOutputStream(file); } @@ -211,12 +201,10 @@ @Override public IndexInput openInput(String name) throws IOException { ensureOpen(); - RAMFile file; - synchronized (this) { - file = fileMap.get(name); - } - if (file == null) + RAMFile file = fileMap.get(name); + if (file == null) { throw new FileNotFoundException(name); + } return new RAMInputStream(file); } @@ -224,6 +212,6 @@ @Override public void close() { isOpen = false; - fileMap = null; + fileMap.clear(); } } Index: lucene/backwards/src/test/org/apache/lucene/store/MockRAMDirectory.java =================================================================== --- lucene/backwards/src/test/org/apache/lucene/store/MockRAMDirectory.java (revision 1040423) +++ lucene/backwards/src/test/org/apache/lucene/store/MockRAMDirectory.java (working copy) @@ -47,6 +47,12 @@ private Set unSyncedFiles; private Set createdFiles; volatile boolean crashed; + + // NOTE: BACKWARDS HACK ! + // fileMap in RAMDir changed from HashMap to Map, and while this change is + // allowed (as it wasn't public API), it breaks the backwards tests. We + // declare a private member here, that is inited by reflection from super's. + private Map fileMap; // NOTE: we cannot initialize the Map here due to the // order in which our constructor actually does this @@ -54,7 +60,17 @@ // like super is called, then our members are initialized: Map openFiles; + @SuppressWarnings("unchecked") private synchronized void init() { + if (fileMap == null) { + try { + // NOTE: BACKWARDS HACK ! + fileMap = (Map) RAMDirectory.class.getDeclaredField("fileMap").get(this); + } catch (Exception e) { + throw new RuntimeException("Inavlid backwards hack", e); + } + } + if (openFiles == null) openFiles = new HashMap(); if (createdFiles == null) @@ -214,9 +230,11 @@ throw new IOException("file " + name + " already exists"); else { if (existing!=null) { - //BACKWARDS BREAK in RamDirectory: sizeInBytes -= existing.sizeInBytes; + // BACKWARDS HACK in RamDirectory: sizeInBytes used to be a long and not + // AtomicLong - however since it wasn't public API, the change was + // allowed, but this breaks this class. try { - ((AtomicLong) getClass().getSuperclass().getDeclaredField("sizeInBytes").get(this)).getAndAdd(-existing.sizeInBytes); + ((AtomicLong) RAMDirectory.class.getDeclaredField("sizeInBytes").get(this)).getAndAdd(-existing.sizeInBytes); } catch (Exception e) { throw new RuntimeException("Backwards-hack failed.", e); }