Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAndrey Loskutov2019-04-28 17:18:50 -0400
committerAndrey Loskutov2019-04-30 06:56:00 -0400
commit4e0281f70e20e78feae8925f35609842163cda70 (patch)
tree8b07bb3743a38702fd959cadcc09b12198eb7c1a
parent370706729e5939ce2b98d8283385374cd4917cfd (diff)
downloadeclipse.jdt.core-4e0281f70e20e78feae8925f35609842163cda70.tar.gz
eclipse.jdt.core-4e0281f70e20e78feae8925f35609842163cda70.tar.xz
eclipse.jdt.core-4e0281f70e20e78feae8925f35609842163cda70.zip
Bug 546319 - ConcurrentModificationException inI20190502-1800I20190501-1800
org.eclipse.jdt.internal.compiler.batch.ClasspathJrt.allModules() Access to module names maps stored in ModulesCache must be synchronized, because it is used from global static context and is not MT safe. Change-Id: I3f6b5e478070a1e2de8d9431fdf43a3513085677 Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
-rw-r--r--org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/ClasspathJep247.java17
-rw-r--r--org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/ClasspathJep247Jdk12.java28
-rw-r--r--org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/ClasspathJrt.java22
3 files changed, 38 insertions, 29 deletions
diff --git a/org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/ClasspathJep247.java b/org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/ClasspathJep247.java
index cd433e2851..0be712927f 100644
--- a/org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/ClasspathJep247.java
+++ b/org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/ClasspathJep247.java
@@ -22,6 +22,7 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -144,6 +145,7 @@ public class ClasspathJep247 extends ClasspathJrt {
Map<String, IModule> cache = ModulesCache.get(this.modulePath);
if (cache == null) {
try (DirectoryStream<java.nio.file.Path> stream = Files.newDirectoryStream(modPath)) {
+ HashMap<String,IModule> newCache = new HashMap<>();
for (final java.nio.file.Path subdir: stream) {
Files.walkFileTree(subdir, new FileVisitor<java.nio.file.Path>() {
@@ -160,7 +162,7 @@ public class ClasspathJep247 extends ClasspathJrt {
content = JRTUtil.safeReadBytes(f);
if (content == null)
return FileVisitResult.CONTINUE;
- ClasspathJep247.this.acceptModule(content);
+ ClasspathJep247.this.acceptModule(content, newCache);
ClasspathJep247.this.moduleNamesCache.add(JRTUtil.sanitizedFileName(f));
}
return FileVisitResult.CONTINUE;
@@ -177,6 +179,11 @@ public class ClasspathJep247 extends ClasspathJrt {
}
});
}
+ synchronized(ModulesCache) {
+ if (ModulesCache.get(this.modulePath) == null) {
+ ModulesCache.put(this.modulePath, Collections.unmodifiableMap(newCache));
+ }
+ }
} catch (IOException e) {
e.printStackTrace();
}
@@ -185,19 +192,15 @@ public class ClasspathJep247 extends ClasspathJrt {
}
}
@Override
- void acceptModule(ClassFileReader reader) {
+ void acceptModule(ClassFileReader reader, Map<String, IModule> cache) {
// Modules below level 8 are not dealt with here. Leave it to ClasspathJrt
if (this.jdklevel <= ClassFileConstants.JDK1_8) {
- super.acceptModule(reader);
+ super.acceptModule(reader, cache);
return;
}
if (reader != null) {
IModule moduleDecl = reader.getModuleDeclaration();
if (moduleDecl != null) {
- Map<String, IModule> cache = ModulesCache.get(this.modulePath);
- if (cache == null) {
- ModulesCache.put(this.modulePath, cache = new HashMap<String,IModule>());
- }
cache.put(String.valueOf(moduleDecl.name()), moduleDecl);
}
}
diff --git a/org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/ClasspathJep247Jdk12.java b/org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/ClasspathJep247Jdk12.java
index 7b6ff4c82c..48a119ad7b 100644
--- a/org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/ClasspathJep247Jdk12.java
+++ b/org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/ClasspathJep247Jdk12.java
@@ -157,6 +157,7 @@ public class ClasspathJep247Jdk12 extends ClasspathJep247 {
this.modules = ModulesCache.get(this.modulePath);
if (this.modules == null) {
try (DirectoryStream<java.nio.file.Path> stream = Files.newDirectoryStream(this.releasePath)) {
+ HashMap<String,IModule> newCache = new HashMap<>();
for (final java.nio.file.Path subdir: stream) {
String rel = JRTUtil.sanitizedFileName(subdir);
if (!rel.contains(this.releaseInHex)) {
@@ -181,7 +182,7 @@ public class ClasspathJep247Jdk12 extends ClasspathJep247 {
return FileVisitResult.CONTINUE;
Path m = f.subpath(1, f.getNameCount() - 1);
String name = JRTUtil.sanitizedFileName(m);
- ClasspathJep247Jdk12.this.acceptModule(name, content);
+ ClasspathJep247Jdk12.this.acceptModule(name, content, newCache);
ClasspathJep247Jdk12.this.moduleNamesCache.add(name);
}
return FileVisitResult.SKIP_SIBLINGS;
@@ -198,6 +199,12 @@ public class ClasspathJep247Jdk12 extends ClasspathJep247 {
}
});
}
+ synchronized(ModulesCache) {
+ if (ModulesCache.get(this.modulePath) == null) {
+ this.modules = Collections.unmodifiableMap(newCache);
+ ModulesCache.put(this.modulePath, this.modules);
+ }
+ }
} catch (IOException e) {
e.printStackTrace();
}
@@ -216,14 +223,12 @@ public class ClasspathJep247Jdk12 extends ClasspathJep247 {
}
return null;
}
- void acceptModule(String name, byte[] content) {
+ void acceptModule(String name, byte[] content, Map<String, IModule> cache) {
if (content == null)
return;
- if (this.modules != null) {
- if (this.modules.containsKey(name))
- return;
- }
+ if (cache.containsKey(name))
+ return;
ClassFileReader reader = null;
try {
@@ -232,23 +237,20 @@ public class ClasspathJep247Jdk12 extends ClasspathJep247 {
e.printStackTrace();
}
if (reader != null) {
- acceptModule(reader);
+ acceptModule(reader, cache);
}
}
@Override
- void acceptModule(ClassFileReader reader) {
+ void acceptModule(ClassFileReader reader, Map<String, IModule> cache) {
// Modules below level 8 are not dealt with here. Leave it to ClasspathJrt
if (this.jdklevel <= ClassFileConstants.JDK1_8) {
- super.acceptModule(reader);
+ super.acceptModule(reader, cache);
return;
}
if (reader != null) {
IModule moduleDecl = reader.getModuleDeclaration();
if (moduleDecl != null) {
- if (this.modules == null) {
- ModulesCache.put(this.modulePath, this.modules = new HashMap<String,IModule>());
- }
- this.modules.put(String.valueOf(moduleDecl.name()), moduleDecl);
+ cache.put(String.valueOf(moduleDecl.name()), moduleDecl);
}
}
}
diff --git a/org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/ClasspathJrt.java b/org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/ClasspathJrt.java
index 117ba4758f..c34ac10029 100644
--- a/org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/ClasspathJrt.java
+++ b/org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/ClasspathJrt.java
@@ -21,6 +21,7 @@ import java.nio.file.Path;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -203,6 +204,7 @@ public class ClasspathJrt extends ClasspathLocation implements IMultiModuleEntry
if (cache == null) {
try {
+ HashMap<String,IModule> newCache = new HashMap<>();
org.eclipse.jdt.internal.compiler.util.JRTUtil.walkModuleImage(this.file,
new org.eclipse.jdt.internal.compiler.util.JRTUtil.JrtFileVisitor<Path>() {
@@ -221,7 +223,7 @@ public class ClasspathJrt extends ClasspathLocation implements IMultiModuleEntry
@Override
public FileVisitResult visitModule(Path p, String name) throws IOException {
try {
- ClasspathJrt.this.acceptModule(JRTUtil.getClassfileContent(ClasspathJrt.this.file, IModule.MODULE_INFO_CLASS, name));
+ ClasspathJrt.this.acceptModule(JRTUtil.getClassfileContent(ClasspathJrt.this.file, IModule.MODULE_INFO_CLASS, name), newCache);
ClasspathJrt.this.moduleNamesCache.add(name);
} catch (ClassFormatException e) {
e.printStackTrace();
@@ -229,6 +231,12 @@ public class ClasspathJrt extends ClasspathLocation implements IMultiModuleEntry
return FileVisitResult.SKIP_SUBTREE;
}
}, JRTUtil.NOTIFY_MODULES);
+
+ synchronized(ModulesCache) {
+ if (ModulesCache.get(this.file.getPath()) == null) {
+ ModulesCache.put(this.file.getPath(), Collections.unmodifiableMap(newCache));
+ }
+ }
} catch (IOException e) {
// TODO: Java 9 Should report better
}
@@ -236,20 +244,16 @@ public class ClasspathJrt extends ClasspathLocation implements IMultiModuleEntry
this.moduleNamesCache.addAll(cache.keySet());
}
}
- void acceptModule(ClassFileReader reader) {
+ void acceptModule(ClassFileReader reader, Map<String, IModule> cache) {
if (reader != null) {
IModule moduleDecl = reader.getModuleDeclaration();
if (moduleDecl != null) {
- Map<String, IModule> cache = ModulesCache.get(this.file.getPath());
- if (cache == null) {
- ModulesCache.put(this.file.getPath(), cache = new HashMap<String,IModule>());
- }
cache.put(String.valueOf(moduleDecl.name()), moduleDecl);
}
}
-
}
- void acceptModule(byte[] content) {
+
+ void acceptModule(byte[] content, Map<String, IModule> cache) {
if (content == null)
return;
ClassFileReader reader = null;
@@ -259,7 +263,7 @@ public class ClasspathJrt extends ClasspathLocation implements IMultiModuleEntry
e.printStackTrace();
}
if (reader != null) {
- acceptModule(reader);
+ acceptModule(reader, cache);
}
}

Back to the top