0

I know there is a concurrency issue here in org.apache.bcel.util.SyntheticRepository and eventhough I know I happen to get ConcurrentModificationException, I cannot reproduce it by unit test.

public class SyntheticRepository extends MemorySensitiveClassPathRepository {
    // This isn't thread-safe
    private static final Map<ClassPath, SyntheticRepository> MAP = new HashMap<>();
   
    public static SyntheticRepository getInstance() {
        return getInstance(ClassPath.SYSTEM_CLASS_PATH);
    }

    public static SyntheticRepository getInstance(final ClassPath classPath) {
        return MAP.computeIfAbsent(classPath, SyntheticRepository::new);
    }

    private SyntheticRepository(final ClassPath path) {
        super(path);
    }
}

Exception stack trace:

java.util.ConcurrentModificationException
    at java.base/java.util.HashMap.computeIfAbsent(HashMap.java:1221)
    at org.apache.bcel.util.SyntheticRepository.getInstance(SyntheticRepository.java:42)
    at org.apache.bcel.util.SyntheticRepository.getInstance(SyntheticRepository.java:38)
    at org.apache.bcel.classfile.JavaClass.<init>(JavaClass.java:145)
    at org.apache.bcel.classfile.ClassParser.parse(ClassParser.java:178)

My unit test is as follows and it always succeeds instead of failing:

import org.apache.bcel.classfile.ClassParser;
import org.apache.bcel.classfile.JavaClass;
import org.apache.bcel.classfile.Utility;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;

public class MultiThreadedBCELTest {

    private final ConcurrentHashMap<String, String> map = new ConcurrentHashMap<>();

    @Test
    public void testConcurrentModification() throws InterruptedException {
        final List<String> classes = Arrays.asList("java.lang.String", "java.util.List", "java.util.Map", "java.time.LocalDate", "java.time.LocalDateTime", "java.math.BigDecimal",
                "java.io.File", "java.net.URL", "java.nio.file.Path", "java.util.concurrent.ConcurrentHashMap", "java.util.regex.Pattern", "java.util.stream.Stream",
                "java.lang.Integer", "java.lang.Double", "java.lang.Boolean", "java.util.ArrayList", "java.util.HashMap", "java.util.Optional", "java.util.Set",
                "java.nio.charset.Charset", "java.time.LocalTime", "java.util.Date", "java.util.LinkedList", "java.util.TreeMap", "java.util.function.Predicate",
                "java.util.function.Function", "java.util.function.Consumer", "java.util.concurrent.CopyOnWriteArrayList", "java.util.concurrent.locks.ReentrantLock",
                "java.lang.Long", "java.lang.Float", "java.lang.Character", "java.util.Collections", "java.util.Comparator", "java.util.Arrays",
                "java.util.concurrent.ConcurrentLinkedQueue", "java.util.concurrent.BlockingQueue", "java.util.concurrent.ScheduledExecutorService",
                "java.util.concurrent.TimeUnit", "java.util.concurrent.atomic.AtomicInteger", "java.util.concurrent.CountDownLatch", "java.util.concurrent.Semaphore",
                "java.util.concurrent.ExecutorService", "java.util.concurrent.Executors", "java.util.concurrent.ThreadPoolExecutor", "java.util.concurrent.CyclicBarrier",
                "java.util.concurrent.DelayQueue", "java.util.concurrent.PriorityBlockingQueue");
        final ExecutorService threadPool = Executors.newFixedThreadPool(20);
        final List<Callable<String>> tasks = new ArrayList<>();
        for (final String className : classes) {
            tasks.add(() -> map.computeIfAbsent(className, MultiThreadedBCELTest::parse));
        }
        final List<Future<String>> results = threadPool.invokeAll(tasks);
        for (final Future<String> future : results) {
            try {
                final String classString = future.get();
                Assertions.assertNotNull(classString);
            } catch (InterruptedException | ExecutionException e) {
                Assertions.fail(e);
            }
        }
    }

    private static String parse(final String entryName) {
        final String className = Utility.packageToPath(entryName) + JavaClass.EXTENSION;
        try (InputStream inputStream = ClassLoader.getSystemResourceAsStream(className)) {
            if (inputStream != null) {
                final String fileName = className.substring(className.lastIndexOf('/') + 1);
                return new ClassParser(inputStream, fileName).parse().toString();
            }
        } catch (final IOException e) {
            Assertions.fail(e);
        }
        return null;
    }
}

Dependencies:

    <dependencies>
        <dependency>
            <groupId>org.apache.bcel</groupId>
            <artifactId>bcel</artifactId>
            <version>6.7.0</version>
        </dependency>
        <dependency>
            <groupId>org.junit.jupiter</groupId>
            <artifactId>junit-jupiter</artifactId>
            <version>5.8.2</version>
            <scope>test</scope>
        </dependency>
    </dependencies>
Sybuser
  • 735
  • 10
  • 27
  • 1
    There is no guaranty that concurrency issues will exhibit a specific behavior. The check within `computeIfAbsent` which could lead to a `ConcurrentModificationException` is made on a best effort basis and does not even exist in all Java versions (most notably, it doesn’t exist in Java 8). Instead of only checking for this exception, you may verify whether you consistently receive the same value for the same key (if not, there were lost updates). But as said, there is no guaranty for a specific misbehavior. It may even happen to do what it is supposed to do in some runs, despite being broken. – Holger Mar 06 '23 at 15:38

0 Answers0