Skip to content

Commit

Permalink
Improve handling of closed and re-opened JAR classpath files
Browse files Browse the repository at this point in the history 
ClasspathJar.openArchives needs more safeguards when accessing possibly
closed zip files (actually classpath JARs), making sure the get
re-opened by calling ClasspathJar.ensureOpen().

Relates to eclipse-aspectj/aspectj#125.

Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Change-Id: If20bae3d6169559de97febc529465196367524b9
  • Loading branch information
@kriegaex
kriegaex committed Feb 23, 2022
1 parent f2d2cd7 commit b958c49
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 26 deletions.
93 changes: 67 additions & 26 deletions org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/ClasspathJar.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ public class ClasspathJar extends ClasspathLocation {
* open and manage the ZipFile.
*/
private static int maxOpenArchives = 1000;
private final static int MAXOPEN_DEFAULT = 1000;
private static List openArchives = new ArrayList();
private static final int MAXOPEN_DEFAULT = 1000;
private static final List<ClasspathJar> openArchives = new ArrayList<>();
// End AspectJ Extension

protected File file;
Expand Down Expand Up @@ -103,6 +103,14 @@ public List<Classpath> fetchLinkedJars(FileSystem.ClasspathSectionProblemReporte
try {
initialize();
ArrayList<Classpath> result = new ArrayList<>();
// AspectJ Extension
try {
ensureOpen();
}
catch (IOException e) {
throw new RuntimeException(e);
}
// End AspectJ Extension
ZipEntry manifest = this.zipFile.getEntry(TypeConstants.META_INF_MANIFEST_MF);
if (manifest != null) { // non-null implies regular file
inputStream = this.zipFile.getInputStream(manifest);
Expand Down Expand Up @@ -144,15 +152,30 @@ public List<Classpath> fetchLinkedJars(FileSystem.ClasspathSectionProblemReporte
}
@Override
public NameEnvironmentAnswer findClass(char[] typeName, String qualifiedPackageName, String moduleName, String qualifiedBinaryFileName) {
// AspectJ Extension
try {
ensureOpen();
}
catch (IOException e) {
throw new RuntimeException(e);
}
// End AspectJ Extension
return findClass(typeName, qualifiedPackageName, moduleName, qualifiedBinaryFileName, false);
}
@Override
public NameEnvironmentAnswer findClass(char[] typeName, String qualifiedPackageName, String moduleName, String qualifiedBinaryFileName, boolean asBinaryOnly) {
if (!isPackage(qualifiedPackageName, moduleName))
return null; // most common case

// AspectJ Extension
try {
ensureOpen();
}
catch (IOException e) {
throw new RuntimeException(e);
}
// End AspectJ Extension
try {
ensureOpen(); // AspectJ Extension
IBinaryType reader = ClassFileReader.read(this.zipFile, qualifiedBinaryFileName);
if (reader != null) {
char[] modName = this.module == null ? null : this.module.name();
Expand Down Expand Up @@ -194,6 +217,14 @@ public NameEnvironmentAnswer findClass(char[] typeName, String qualifiedPackageN
public boolean hasAnnotationFileFor(String qualifiedTypeName) {
if (this.zipFile == null)
return false;
// AspectJ Extension
try {
ensureOpen();
}
catch (IOException e) {
throw new RuntimeException(e);
}
// End AspectJ Extension
return this.zipFile.getEntry(qualifiedTypeName+ExternalAnnotationProvider.ANNOTATION_FILE_SUFFIX) != null;
}
@Override
Expand All @@ -202,18 +233,14 @@ public char[][][] findTypeNames(final String qualifiedPackageName, String module
return null; // most common case
final char[] packageArray = qualifiedPackageName.toCharArray();
final ArrayList answers = new ArrayList();

// AspectJ Extension
try {
ensureOpen();
} catch (IOException ioe) {
// Doesn't normally occur - probably means since starting the compile
// you have removed one of the jars.
ioe.printStackTrace();
return null;
ensureOpen();
}
catch (IOException e) {
throw new RuntimeException(e);
}
// End AspectJ Extension

nextEntry : for (Enumeration e = this.zipFile.entries(); e.hasMoreElements(); ) {
String fileName = ((ZipEntry) e.nextElement()).getName();

Expand Down Expand Up @@ -285,15 +312,12 @@ protected void addToPackageCache(String fileName, boolean endsWithSep) {
public synchronized char[][] getModulesDeclaringPackage(String qualifiedPackageName, String moduleName) {
if (this.packageCache != null)
return singletonModuleNameIf(this.packageCache.contains(qualifiedPackageName));

// AspectJ Extension
try {
ensureOpen();
} catch (IOException ioe) {
// Doesn't normally occur - probably means since starting the compile
// you have removed one of the jars.
ioe.printStackTrace();
return singletonModuleNameIf(false);
ensureOpen();
}
catch (IOException e) {
throw new RuntimeException(e);
}
// End AspectJ Extension
this.packageCache = new HashSet<>(41);
Expand All @@ -308,6 +332,14 @@ public synchronized char[][] getModulesDeclaringPackage(String qualifiedPackageN
@Override
public boolean hasCompilationUnit(String qualifiedPackageName, String moduleName) {
qualifiedPackageName += '/';
// AspectJ Extension
try {
ensureOpen();
}
catch (IOException e) {
throw new RuntimeException(e);
}
// End AspectJ Extension
for (Enumeration<? extends ZipEntry> e = this.zipFile.entries(); e.hasMoreElements(); ) {
String fileName = e.nextElement().getName();
if (fileName.startsWith(qualifiedPackageName) && fileName.length() > qualifiedPackageName.length()) {
Expand All @@ -324,6 +356,14 @@ public boolean hasCompilationUnit(String qualifiedPackageName, String moduleName
@Override
public char[][] listPackages() {
Set<String> packageNames = new HashSet<>();
// AspectJ Extension
try {
ensureOpen();
}
catch (IOException e) {
throw new RuntimeException(e);
}
// End AspectJ Extension
for (Enumeration<? extends ZipEntry> e = this.zipFile.entries(); e.hasMoreElements(); ) {
String fileName = e.nextElement().getName();
int lastSlash = fileName.lastIndexOf('/');
Expand Down Expand Up @@ -399,20 +439,21 @@ public int getMode() {
public IModule getModule() {
return this.module;
}

// AspectJ Extension
private void ensureOpen() throws IOException {
if (zipFile != null) return; // If its not null, the zip is already open
if (openArchives.size()>=maxOpenArchives) {
closeSomeArchives(openArchives.size()/10); // Close 10% of those open
protected void ensureOpen() throws IOException {
if (zipFile != null) return; // If it is not null, the zip is already open
final int openArchivesSize = openArchives.size();
if (openArchivesSize >= maxOpenArchives) {
closeSomeArchives(openArchivesSize / 10); // Close 10% of those open
}
zipFile = new ZipFile(file);
openArchives.add(this);
}

private void closeSomeArchives(int n) {
for (int i=n-1;i>=0;i--) {
ClasspathJar zf = (ClasspathJar)openArchives.get(0);
zf.close();
for (int i = n - 1; i >= 0; i--) {
openArchives.get(0).close();
}
}

Expand All @@ -436,6 +477,6 @@ private static String getSystemPropertyWithoutSecurityException (String aPropert
return aDefaultValue;
}
}

// End AspectJ Extension

}
40 changes: 40 additions & 0 deletions org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/ClasspathJmod.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ public NameEnvironmentAnswer findClass(char[] typeName, String qualifiedPackageN
if (!isPackage(qualifiedPackageName, moduleName))
return null; // most common case

// AspectJ Extension
try {
ensureOpen();
}
catch (IOException e) {
throw new RuntimeException(e);
}
// End AspectJ Extension
try {
qualifiedBinaryFileName = new String(CharOperation.append(CLASSES_FOLDER, qualifiedBinaryFileName.toCharArray()));
IBinaryType reader = ClassFileReader.read(this.zipFile, qualifiedBinaryFileName);
Expand Down Expand Up @@ -97,6 +105,14 @@ public NameEnvironmentAnswer findClass(char[] typeName, String qualifiedPackageN
@Override
public boolean hasAnnotationFileFor(String qualifiedTypeName) {
qualifiedTypeName = new String(CharOperation.append(CLASSES_FOLDER, qualifiedTypeName.toCharArray()));
// AspectJ Extension
try {
ensureOpen();
}
catch (IOException e) {
throw new RuntimeException(e);
}
// End AspectJ Extension
return this.zipFile.getEntry(qualifiedTypeName+ExternalAnnotationProvider.ANNOTATION_FILE_SUFFIX) != null;
}
@SuppressWarnings({ "rawtypes", "unchecked" })
Expand All @@ -106,6 +122,14 @@ public char[][][] findTypeNames(final String qualifiedPackageName, String module
return null; // most common case
final char[] packageArray = qualifiedPackageName.toCharArray();
final ArrayList answers = new ArrayList();
// AspectJ Extension
try {
ensureOpen();
}
catch (IOException e) {
throw new RuntimeException(e);
}
// End AspectJ Extension
nextEntry : for (Enumeration e = this.zipFile.entries(); e.hasMoreElements(); ) {
String fileName = ((ZipEntry) e.nextElement()).getName();

Expand Down Expand Up @@ -143,6 +167,14 @@ public synchronized char[][] getModulesDeclaringPackage(String qualifiedPackageN
this.packageCache = new HashSet<>(41);
this.packageCache.add(Util.EMPTY_STRING);

// AspectJ Extension
try {
ensureOpen();
}
catch (IOException e) {
throw new RuntimeException(e);
}
// End AspectJ Extension
for (Enumeration<? extends ZipEntry> e = this.zipFile.entries(); e.hasMoreElements(); ) {
char[] entryName = e.nextElement().getName().toCharArray();
int index = CharOperation.indexOf('/', entryName);
Expand All @@ -159,6 +191,14 @@ public synchronized char[][] getModulesDeclaringPackage(String qualifiedPackageN
@Override
public boolean hasCompilationUnit(String qualifiedPackageName, String moduleName) {
qualifiedPackageName += '/';
// AspectJ Extension
try {
ensureOpen();
}
catch (IOException e) {
throw new RuntimeException(e);
}
// End AspectJ Extension
for (Enumeration<? extends ZipEntry> e = this.zipFile.entries(); e.hasMoreElements(); ) {
char[] entryName = e.nextElement().getName().toCharArray();
int index = CharOperation.indexOf('/', entryName);
Expand Down
8 changes: 8 additions & 0 deletions org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/ClasspathSourceJar.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ public NameEnvironmentAnswer findClass(char[] typeName, String qualifiedPackageN
if (!isPackage(qualifiedPackageName, moduleName))
return null; // most common case

// AspectJ Extension
try {
ensureOpen();
}
catch (IOException e) {
throw new RuntimeException(e);
}
// End AspectJ Extension
ZipEntry sourceEntry = this.zipFile.getEntry(qualifiedBinaryFileName.substring(0, qualifiedBinaryFileName.length() - 6) + SUFFIX_STRING_java);
if (sourceEntry != null) {
try {
Expand Down
10 changes: 10 additions & 0 deletions org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/FileSystem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,16 @@ private NameEnvironmentAnswer findClass(String qualifiedTypeName, char[] typeNam
if (classpathEntry.hasAnnotationFileFor(qualifiedTypeName)) {
// in case of 'this.annotationsFromClasspath' we indeed search for .eea entries inside the main zipFile of the entry:
ZipFile zip = classpathEntry instanceof ClasspathJar ? ((ClasspathJar) classpathEntry).zipFile : null;
// AspectJ Extension
if (classpathEntry instanceof ClasspathJar) {
try {
((ClasspathJar) classpathEntry).ensureOpen();
}
catch (IOException e) {
throw new RuntimeException(e);
}
}
// End AspectJ Extension
boolean shouldClose = false; // don't close classpathEntry.zipFile, which we don't own
try {
if (zip == null) {
Expand Down

0 comments on commit b958c49

Please sign in to comment.