# HG changeset patch # User jjg # Date 1255560088 25200 # Node ID 9788f4549740b4d7783e47bdc9fdd199b94fce58 # Parent 70eaf9773f81f12b7c81ed57e9c7280962d9365b 6838467: JSR199 FileObjects don't obey general contract of equals. Reviewed-by: darcy diff -r 70eaf9773f81 -r 9788f4549740 langtools/src/share/classes/com/sun/tools/javac/file/BaseFileObject.java --- a/langtools/src/share/classes/com/sun/tools/javac/file/BaseFileObject.java Tue Oct 13 15:26:30 2009 -0700 +++ b/langtools/src/share/classes/com/sun/tools/javac/file/BaseFileObject.java Wed Oct 14 15:41:28 2009 -0700 @@ -120,6 +120,14 @@ } + // force subtypes to define equals + @Override + public abstract boolean equals(Object other); + + // force subtypes to define hashCode + @Override + public abstract int hashCode(); + /** The file manager that created this JavaFileObject. */ protected final JavacFileManager fileManager; } diff -r 70eaf9773f81 -r 9788f4549740 langtools/src/share/classes/com/sun/tools/javac/file/JavacFileManager.java --- a/langtools/src/share/classes/com/sun/tools/javac/file/JavacFileManager.java Tue Oct 13 15:26:30 2009 -0700 +++ b/langtools/src/share/classes/com/sun/tools/javac/file/JavacFileManager.java Wed Oct 14 15:41:28 2009 -0700 @@ -968,7 +968,7 @@ } else { File siblingDir = null; if (sibling != null && sibling instanceof RegularFileObject) { - siblingDir = ((RegularFileObject)sibling).f.getParentFile(); + siblingDir = ((RegularFileObject)sibling).file.getParentFile(); } return new RegularFileObject(this, new File(siblingDir, fileName.basename())); } diff -r 70eaf9773f81 -r 9788f4549740 langtools/src/share/classes/com/sun/tools/javac/file/RegularFileObject.java --- a/langtools/src/share/classes/com/sun/tools/javac/file/RegularFileObject.java Tue Oct 13 15:26:30 2009 -0700 +++ b/langtools/src/share/classes/com/sun/tools/javac/file/RegularFileObject.java Wed Oct 14 15:41:28 2009 -0700 @@ -33,6 +33,8 @@ import java.io.OutputStream; import java.io.OutputStreamWriter; import java.io.Writer; +import java.lang.ref.Reference; +import java.lang.ref.SoftReference; import java.net.URI; import java.nio.ByteBuffer; import java.nio.CharBuffer; @@ -53,7 +55,8 @@ */ private boolean hasParents = false; private String name; - final File f; + final File file; + private Reference absFileRef; public RegularFileObject(JavacFileManager fileManager, File f) { this(fileManager, f.getName(), f); @@ -65,17 +68,17 @@ throw new IllegalArgumentException("directories not supported"); } this.name = name; - this.f = f; + this.file = f; } @Override public URI toUri() { - return f.toURI().normalize(); + return file.toURI().normalize(); } @Override public String getName() { - return f.getPath(); + return file.getPath(); } @Override @@ -90,20 +93,20 @@ @Override public InputStream openInputStream() throws IOException { - return new FileInputStream(f); + return new FileInputStream(file); } @Override public OutputStream openOutputStream() throws IOException { ensureParentDirectoriesExist(); - return new FileOutputStream(f); + return new FileOutputStream(file); } @Override public CharBuffer getCharContent(boolean ignoreEncodingErrors) throws IOException { CharBuffer cb = fileManager.getCachedContent(this); if (cb == null) { - InputStream in = new FileInputStream(f); + InputStream in = new FileInputStream(file); try { ByteBuffer bb = fileManager.makeByteBuffer(in); JavaFileObject prev = fileManager.log.useSource(this); @@ -126,17 +129,17 @@ @Override public Writer openWriter() throws IOException { ensureParentDirectoriesExist(); - return new OutputStreamWriter(new FileOutputStream(f), fileManager.getEncodingName()); + return new OutputStreamWriter(new FileOutputStream(file), fileManager.getEncodingName()); } @Override public long getLastModified() { - return f.lastModified(); + return file.lastModified(); } @Override public boolean delete() { - return f.delete(); + return file.delete(); } @Override @@ -146,7 +149,7 @@ @Override protected String inferBinaryName(Iterable path) { - String fPath = f.getPath(); + String fPath = file.getPath(); //System.err.println("RegularFileObject " + file + " " +r.getPath()); for (File dir: path) { //System.err.println("dir: " + dir); @@ -178,7 +181,7 @@ if (name.equalsIgnoreCase(n)) { try { // allow for Windows - return f.getCanonicalFile().getName().equals(n); + return file.getCanonicalFile().getName().equals(n); } catch (IOException e) { } } @@ -187,7 +190,7 @@ private void ensureParentDirectoriesExist() throws IOException { if (!hasParents) { - File parent = f.getParentFile(); + File parent = file.getParentFile(); if (parent != null && !parent.exists()) { if (!parent.mkdirs()) { if (!parent.exists() || !parent.isDirectory()) { @@ -199,21 +202,34 @@ } } + /** + * Check if two file objects are equal. + * Two RegularFileObjects are equal if the absolute paths of the underlying + * files are equal. + */ @Override public boolean equals(Object other) { - if (!(other instanceof RegularFileObject)) { + if (this == other) + return true; + + if (!(other instanceof RegularFileObject)) return false; - } + RegularFileObject o = (RegularFileObject) other; - try { - return f.equals(o.f) || f.getCanonicalFile().equals(o.f.getCanonicalFile()); - } catch (IOException e) { - return false; - } + return getAbsoluteFile().equals(o.getAbsoluteFile()); } @Override public int hashCode() { - return f.hashCode(); + return getAbsoluteFile().hashCode(); + } + + private File getAbsoluteFile() { + File absFile = (absFileRef == null ? null : absFileRef.get()); + if (absFile == null) { + absFile = file.getAbsoluteFile(); + absFileRef = new SoftReference(absFile); + } + return absFile; } } diff -r 70eaf9773f81 -r 9788f4549740 langtools/src/share/classes/com/sun/tools/javac/file/SymbolArchive.java --- a/langtools/src/share/classes/com/sun/tools/javac/file/SymbolArchive.java Tue Oct 13 15:26:30 2009 -0700 +++ b/langtools/src/share/classes/com/sun/tools/javac/file/SymbolArchive.java Wed Oct 14 15:41:28 2009 -0700 @@ -76,13 +76,13 @@ @Override public JavaFileObject getFileObject(RelativeDirectory subdirectory, String file) { RelativeDirectory prefix_subdir = new RelativeDirectory(prefix, subdirectory.path); - ZipEntry ze = new RelativeFile(prefix_subdir, file).getZipEntry(zdir); + ZipEntry ze = new RelativeFile(prefix_subdir, file).getZipEntry(zfile); return new SymbolFileObject(this, file, ze); } @Override public String toString() { - return "SymbolArchive[" + zdir.getName() + "]"; + return "SymbolArchive[" + zfile.getName() + "]"; } /** diff -r 70eaf9773f81 -r 9788f4549740 langtools/src/share/classes/com/sun/tools/javac/file/ZipArchive.java --- a/langtools/src/share/classes/com/sun/tools/javac/file/ZipArchive.java Tue Oct 13 15:26:30 2009 -0700 +++ b/langtools/src/share/classes/com/sun/tools/javac/file/ZipArchive.java Wed Oct 14 15:41:28 2009 -0700 @@ -47,6 +47,8 @@ import com.sun.tools.javac.file.RelativePath.RelativeDirectory; import com.sun.tools.javac.file.RelativePath.RelativeFile; import com.sun.tools.javac.util.List; +import java.lang.ref.Reference; +import java.lang.ref.SoftReference; /** *

This is NOT part of any API supported by Sun Microsystems. @@ -56,20 +58,20 @@ */ public class ZipArchive implements Archive { - public ZipArchive(JavacFileManager fm, ZipFile zdir) throws IOException { - this(fm, zdir, true); + public ZipArchive(JavacFileManager fm, ZipFile zfile) throws IOException { + this(fm, zfile, true); } - protected ZipArchive(JavacFileManager fm, ZipFile zdir, boolean initMap) throws IOException { + protected ZipArchive(JavacFileManager fm, ZipFile zfile, boolean initMap) throws IOException { this.fileManager = fm; - this.zdir = zdir; + this.zfile = zfile; this.map = new HashMap>(); if (initMap) initMap(); } protected void initMap() throws IOException { - for (Enumeration e = zdir.entries(); e.hasMoreElements(); ) { + for (Enumeration e = zfile.entries(); e.hasMoreElements(); ) { ZipEntry entry; try { entry = e.nextElement(); @@ -110,7 +112,7 @@ } public JavaFileObject getFileObject(RelativeDirectory subdirectory, String file) { - ZipEntry ze = new RelativeFile(subdirectory, file).getZipEntry(zdir); + ZipEntry ze = new RelativeFile(subdirectory, file).getZipEntry(zfile); return new ZipFileObject(this, file, ze); } @@ -119,17 +121,39 @@ } public void close() throws IOException { - zdir.close(); + zfile.close(); } @Override public String toString() { - return "ZipArchive[" + zdir.getName() + "]"; + return "ZipArchive[" + zfile.getName() + "]"; + } + + private File getAbsoluteFile() { + File absFile = (absFileRef == null ? null : absFileRef.get()); + if (absFile == null) { + absFile = new File(zfile.getName()).getAbsoluteFile(); + absFileRef = new SoftReference(absFile); + } + return absFile; } + /** + * The file manager that created this archive. + */ protected JavacFileManager fileManager; + /** + * The index for the contents of this archive. + */ protected final Map> map; - protected final ZipFile zdir; + /** + * The zip file for the archive. + */ + protected final ZipFile zfile; + /** + * A reference to the absolute filename for the zip file for the archive. + */ + protected Reference absFileRef; /** * A subclass of JavaFileObject representing zip entries. @@ -148,18 +172,18 @@ } public URI toUri() { - File zipFile = new File(zarch.zdir.getName()); + File zipFile = new File(zarch.zfile.getName()); return createJarUri(zipFile, entry.getName()); } @Override public String getName() { - return zarch.zdir.getName() + "(" + entry.getName() + ")"; + return zarch.zfile.getName() + "(" + entry.getName() + ")"; } @Override public String getShortName() { - return new File(zarch.zdir.getName()).getName() + "(" + entry + ")"; + return new File(zarch.zfile.getName()).getName() + "(" + entry + ")"; } @Override @@ -169,7 +193,7 @@ @Override public InputStream openInputStream() throws IOException { - return zarch.zdir.getInputStream(entry); + return zarch.zfile.getInputStream(entry); } @Override @@ -181,7 +205,7 @@ public CharBuffer getCharContent(boolean ignoreEncodingErrors) throws IOException { CharBuffer cb = fileManager.getCachedContent(this); if (cb == null) { - InputStream in = zarch.zdir.getInputStream(entry); + InputStream in = zarch.zfile.getInputStream(entry); try { ByteBuffer bb = fileManager.makeByteBuffer(in); JavaFileObject prev = fileManager.log.useSource(this); @@ -237,18 +261,27 @@ return name.equals(cn + k.extension); } + /** + * Check if two file objects are equal. + * Two ZipFileObjects are equal if the absolute paths of the underlying + * zip files are equal and if the paths within those zip files are equal. + */ @Override public boolean equals(Object other) { - if (!(other instanceof ZipFileObject)) { + if (this == other) + return true; + + if (!(other instanceof ZipFileObject)) return false; - } + ZipFileObject o = (ZipFileObject) other; - return zarch.zdir.equals(o.zarch.zdir) || name.equals(o.name); + return zarch.getAbsoluteFile().equals(o.zarch.getAbsoluteFile()) + && name.equals(o.name); } @Override public int hashCode() { - return zarch.zdir.hashCode() + name.hashCode(); + return zarch.getAbsoluteFile().hashCode() + name.hashCode(); } } diff -r 70eaf9773f81 -r 9788f4549740 langtools/src/share/classes/com/sun/tools/javac/file/ZipFileIndex.java --- a/langtools/src/share/classes/com/sun/tools/javac/file/ZipFileIndex.java Tue Oct 13 15:26:30 2009 -0700 +++ b/langtools/src/share/classes/com/sun/tools/javac/file/ZipFileIndex.java Wed Oct 14 15:41:28 2009 -0700 @@ -30,6 +30,7 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.RandomAccessFile; +import java.lang.ref.Reference; import java.lang.ref.SoftReference; import java.util.ArrayList; import java.util.Arrays; @@ -89,6 +90,7 @@ // ZipFileIndex data entries private File zipFile; + private Reference absFileRef; private long zipFileLastModified = NOT_MODIFIED; private RandomAccessFile zipRandomFile; private Entry[] entries; @@ -1215,6 +1217,15 @@ return zipFile; } + File getAbsoluteFile() { + File absFile = (absFileRef == null ? null : absFileRef.get()); + if (absFile == null) { + absFile = zipFile.getAbsoluteFile(); + absFileRef = new SoftReference(absFile); + } + return absFile; + } + private RelativeDirectory getRelativeDirectory(String path) { RelativeDirectory rd; SoftReference ref = relativeDirectoryCache.get(path); diff -r 70eaf9773f81 -r 9788f4549740 langtools/src/share/classes/com/sun/tools/javac/file/ZipFileIndexArchive.java --- a/langtools/src/share/classes/com/sun/tools/javac/file/ZipFileIndexArchive.java Tue Oct 13 15:26:30 2009 -0700 +++ b/langtools/src/share/classes/com/sun/tools/javac/file/ZipFileIndexArchive.java Wed Oct 14 15:41:28 2009 -0700 @@ -219,17 +219,27 @@ return name.equals(cn + k.extension); } + /** + * Check if two file objects are equal. + * Two ZipFileIndexFileObjects are equal if the absolute paths of the underlying + * zip files are equal and if the paths within those zip files are equal. + */ @Override public boolean equals(Object other) { + if (this == other) + return true; + if (!(other instanceof ZipFileIndexFileObject)) return false; + ZipFileIndexFileObject o = (ZipFileIndexFileObject) other; - return entry.equals(o.entry); + return zfIndex.getAbsoluteFile().equals(o.zfIndex.getAbsoluteFile()) + && name.equals(o.name); } @Override public int hashCode() { - return zipName.hashCode() + (name.hashCode() << 10); + return zfIndex.getAbsoluteFile().hashCode() + name.hashCode(); } private String getPrefixedEntryName() { diff -r 70eaf9773f81 -r 9788f4549740 langtools/src/share/classes/com/sun/tools/javac/jvm/ClassReader.java --- a/langtools/src/share/classes/com/sun/tools/javac/jvm/ClassReader.java Tue Oct 13 15:26:30 2009 -0700 +++ b/langtools/src/share/classes/com/sun/tools/javac/jvm/ClassReader.java Wed Oct 14 15:41:28 2009 -0700 @@ -2632,10 +2632,20 @@ return true; // fail-safe mode } + /** + * Check if two file objects are equal. + * SourceFileObjects are just placeholder objects for the value of a + * SourceFile attribute, and do not directly represent specific files. + * Two SourceFileObjects are equal if their names are equal. + */ @Override public boolean equals(Object other) { + if (this == other) + return true; + if (!(other instanceof SourceFileObject)) return false; + SourceFileObject o = (SourceFileObject) other; return name.equals(o.name); } diff -r 70eaf9773f81 -r 9788f4549740 langtools/test/tools/javac/api/6440528/T6440528.java --- a/langtools/test/tools/javac/api/6440528/T6440528.java Tue Oct 13 15:26:30 2009 -0700 +++ b/langtools/test/tools/javac/api/6440528/T6440528.java Wed Oct 14 15:41:28 2009 -0700 @@ -59,9 +59,9 @@ } private File getUnderlyingFile(Object o) throws Exception { - Field f = o.getClass().getDeclaredField("f"); - f.setAccessible(true); - return (File)f.get(o); + Field file = o.getClass().getDeclaredField("file"); + file.setAccessible(true); + return (File)file.get(o); } public static void main(String... args) throws Exception { diff -r 70eaf9773f81 -r 9788f4549740 langtools/test/tools/javac/api/T6838467.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/langtools/test/tools/javac/api/T6838467.java Wed Oct 14 15:41:28 2009 -0700 @@ -0,0 +1,249 @@ +/* + * Copyright 2009 Sun Microsystems, Inc. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara, + * CA 95054 USA or visit www.sun.com if you need additional information or + * have any questions. + */ + +/* + * @test + * @bug 6838467 + * @summary JSR199 FileObjects don't obey general contract of equals. + */ + +import java.io.*; +import java.util.*; +import java.util.zip.*; +import javax.tools.*; +import com.sun.tools.javac.file.JavacFileManager; +import com.sun.tools.javac.util.Context; + +public class T6838467 { + boolean fileSystemIsCaseSignificant = !new File("a").equals(new File("A")); + + enum FileKind { + DIR("dir"), + ZIP("zip"), + ZIPFILEINDEX("zip"); + FileKind(String path) { + file = new File(path); + } + final File file; + }; + + enum CompareKind { + SAME { + File other(File f) { return f; } + }, + ABSOLUTE { + File other(File f) { return f.getAbsoluteFile(); } + }, + DIFFERENT { + File other(File f) { return new File("not_" + f.getPath()); } + }, + CASEEQUIV { + File other(File f) { return new File(f.getPath().toUpperCase()); } + }; + abstract File other(File f); + }; + + String[] paths = { "p/A.java", "p/B.java", "p/C.java" }; + + public static void main(String... args) throws Exception { + new T6838467().run(); + } + + void run() throws Exception { + // on Windows, verify file system is not case significant + if (System.getProperty("os.name").toLowerCase().startsWith("windows") + && fileSystemIsCaseSignificant) { + error("fileSystemIsCaseSignificant is set on Windows."); + } + + // create a set of directories and zip files to compare + createTestDir(new File("dir"), paths); + createTestDir(new File("not_dir"), paths); + createTestZip(new File("zip"), paths); + createTestZip(new File("not_zip"), paths); + if (fileSystemIsCaseSignificant) { + createTestDir(new File("DIR"), paths); + createTestZip(new File("ZIP"), paths); + } + + // test the various sorts of file objects that can be obtained from + // the file manager, and for various values that may or may not match. + for (FileKind fk: FileKind.values()) { + for (CompareKind ck: CompareKind.values()) { + test(fk, ck); + } + } + + // verify that the various different types of file object were all + // tested + Set expectClasses = new HashSet(Arrays.asList( + "RegularFileObject", "ZipFileObject", "ZipFileIndexFileObject" )); + if (!foundClasses.equals(expectClasses)) { + error("expected fileobject classes not found\n" + + "expected: " + expectClasses + "\n" + + "found: " + foundClasses); + } + + if (errors > 0) + throw new Exception(errors + " errors"); + } + + void test(FileKind fk, CompareKind ck) throws IOException { + File f1 = fk.file; + JavaFileManager fm1 = createFileManager(fk, f1); + + File f2 = ck.other(fk.file); + JavaFileManager fm2 = createFileManager(fk, f2); + + try { + // If the directories or zip files match, we expect "n" matches in + // the "n-squared" comparisons to come, where "n" is the number of + // entries in the the directories or zip files. + // If the directories or zip files don't themselves match, + // we obviously don't expect any of their contents to match either. + int expect = (f1.getAbsoluteFile().equals(f2.getAbsoluteFile()) ? paths.length : 0); + + System.err.println("test " + (++count) + " " + fk + " " + ck + " " + f1 + " " + f2); + test(fm1, fm2, expect); + + } finally { + fm1.close(); + fm2.close(); + } + } + + // For a pair of file managers that may or may not have similar entries + // on the classpath, compare all files returned from one against all files + // returned from the other. For each pair of files, verify that if they + // are equal, the hashcode is equal as well, and finally verify that the + // expected number of matches was found. + void test(JavaFileManager fm1, JavaFileManager fm2, int expectEqualCount) throws IOException { + boolean foundFiles1 = false; + boolean foundFiles2 = false; + int foundEqualCount = 0; + Set kinds = EnumSet.allOf(JavaFileObject.Kind.class); + for (FileObject fo1: fm1.list(StandardLocation.CLASS_PATH, "p", kinds, false)) { + foundFiles1 = true; + foundClasses.add(fo1.getClass().getSimpleName()); + for (FileObject fo2: fm2.list(StandardLocation.CLASS_PATH, "p", kinds, false)) { + foundFiles2 = true; + foundClasses.add(fo1.getClass().getSimpleName()); + System.err.println("compare " + fo1 + " " + fo2); + if (fo1.equals(fo2)) { + foundEqualCount++; + int hash1 = fo1.hashCode(); + int hash2 = fo2.hashCode(); + if (hash1 != hash2) + error("hashCode error: " + fo1 + " [" + hash1 + "] " + + fo2 + " [" + hash2 + "]"); + } + } + } + if (!foundFiles1) + error("no files found for file manager 1"); + if (!foundFiles2) + error("no files found for file manager 2"); + // verify the expected number of matches were found + if (foundEqualCount != expectEqualCount) + error("expected matches not found: expected " + expectEqualCount + ", found " + foundEqualCount); + } + + // create a file manager to test a FileKind, with a given directory + // or zip file placed on the classpath + JavaFileManager createFileManager(FileKind fk, File classpath) throws IOException { + StandardJavaFileManager fm = createFileManager(fk == FileKind.ZIP); + fm.setLocation(StandardLocation.CLASS_PATH, Arrays.asList(classpath)); + return fm; + } + + JavacFileManager createFileManager(boolean useJavaUtilZip) { + // javac should really not be using system properties like this + // -- it should really be using (hidden) options -- but until then + // take care to leave system properties as we find them, so as not + // to adversely affect other tests that might follow. + String prev = System.getProperty("useJavaUtilZip"); + boolean resetProperties = false; + try { + if (useJavaUtilZip) { + System.setProperty("useJavaUtilZip", "true"); + resetProperties = true; + } else if (System.getProperty("useJavaUtilZip") != null) { + System.getProperties().remove("useJavaUtilZip"); + resetProperties = true; + } + + Context c = new Context(); + return new JavacFileManager(c, false, null); + } finally { + if (resetProperties) { + if (prev == null) { + System.getProperties().remove("useJavaUtilZip"); + } else { + System.setProperty("useJavaUtilZip", prev); + } + } + } + } + + // create a directory containing a given set of paths + void createTestDir(File dir, String[] paths) throws IOException { + for (String p: paths) { + File file = new File(dir, p); + file.getParentFile().mkdirs(); + FileWriter out = new FileWriter(file); + try { + out.write(p); + } finally { + out.close(); + } + } + } + + // create a sip file containing a given set of entries + void createTestZip(File zip, String[] paths) throws IOException { + if (zip.getParentFile() != null) + zip.getParentFile().mkdirs(); + ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(zip)); + try { + for (String p: paths) { + ZipEntry ze = new ZipEntry(p); + zos.putNextEntry(ze); + byte[] bytes = p.getBytes(); + zos.write(bytes, 0, bytes.length); + zos.closeEntry(); + } + } finally { + zos.close(); + } + } + + void error(String msg) { + System.err.println("Error: " + msg); + errors++; + } + + int count; + int errors; + Set foundClasses = new HashSet(); +} +