Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix issue 305: CodeConverter.replaceArrayAccess leads to java.util.ConcurrentModificationException #306

Merged
merged 1 commit into from Apr 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 11 additions & 11 deletions src/main/javassist/bytecode/analysis/Type.java
Expand Up @@ -15,9 +15,7 @@
*/
package javassist.bytecode.analysis;

import java.util.HashMap;
import java.util.IdentityHashMap;
import java.util.Map;
import java.util.*;

import javassist.ClassPool;
import javassist.CtClass;
Expand Down Expand Up @@ -496,24 +494,26 @@ Map<String,CtClass> findCommonInterfaces(Map<String,CtClass> typeMap, Map<String
if (typeMap == null||typeMap.isEmpty())
alterMap.clear();

for (String name:alterMap.keySet())
Iterator<String> it = alterMap.keySet().iterator();
while (it.hasNext()) {
String name = it.next();
if (!typeMap.containsKey(name))
alterMap.remove(name);
it.remove();
}

// Reduce to subinterfaces
// This does not need to be recursive since we make a copy,
// and that copy contains all super types for the whole hierarchy
for (CtClass intf:alterMap.values()) {
CtClass[] interfaces;
Collection<CtClass> interfaces = new ArrayList<>();
for (CtClass intf : alterMap.values()) {
try {
interfaces = intf.getInterfaces();
interfaces.addAll(Arrays.asList(intf.getInterfaces()));
} catch (NotFoundException e) {
throw new RuntimeException(e);
}

for (CtClass c:interfaces)
alterMap.remove(c.getName());
}
for (CtClass c : interfaces)
alterMap.remove(c.getName());

return alterMap;
}
Expand Down
1 change: 1 addition & 0 deletions src/test/javassist/JvstTest.java
Expand Up @@ -1179,6 +1179,7 @@ public static Test suite() {
suite.addTestSuite(test.javassist.proxy.ProxySimpleTest.class);
suite.addTestSuite(test.javassist.bytecode.analysis.AnalyzerTest.class);
suite.addTestSuite(test.javassist.convert.ArrayAccessReplaceTest.class);
suite.addTestSuite(test.javassist.convert.ArrayAccessReplaceTest2.class);
suite.addTestSuite(test.javassist.bytecode.analysis.DomTreeTest.class);
return suite;
}
Expand Down
21 changes: 21 additions & 0 deletions src/test/test/javassist/convert/ArrayAccessReplaceTest2.java
@@ -0,0 +1,21 @@
package test.javassist.convert;

import javassist.ClassPool;
import javassist.CodeConverter;
import javassist.CtClass;
import junit.framework.TestCase;

public class ArrayAccessReplaceTest2 extends TestCase {

public void testAdvancedInstrumentation() throws Exception {
ClassPool pool = new ClassPool(true);
CtClass monitoringClass = pool.get(ArrayAccessReplaceTest2.class.getName());
CtClass targetClass = pool.get(InstrumentationTarget.class.getName());
CodeConverter converter = new CodeConverter();
// we just test if the instrumentation works, the monitoring class does not need to actually contain the replacer methods
// what is only relevant when code gets executed
converter.replaceArrayAccess(monitoringClass, new CodeConverter.DefaultArrayAccessReplacementMethodNames());
targetClass.instrument(converter);
}

}