From 3dc1ca7f4f0682f12e5023d97f79a9aaed60ecea Mon Sep 17 00:00:00 2001
From: Guillaume Nodet <gnodet@gmail.com>
Date: Sat, 28 Oct 2023 11:32:32 +0200
Subject: [PATCH] Improve locking on JavacCompiler

- the code synchonizes on the CopyOnWriteArrayList which is a concurrent collection
- the double check locking pattern is badly written and does not actually double checks
---
 .../plexus/compiler/javac/JavacCompiler.java  | 37 +++++++++----------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/plexus-compilers/plexus-compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavacCompiler.java b/plexus-compilers/plexus-compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavacCompiler.java
index dbee1de9..e78ecbbc 100644
--- a/plexus-compilers/plexus-compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavacCompiler.java
+++ b/plexus-compilers/plexus-compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavacCompiler.java
@@ -58,12 +58,13 @@
 import java.net.URLClassLoader;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Deque;
 import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Properties;
 import java.util.StringTokenizer;
-import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ConcurrentLinkedDeque;
 
 import org.codehaus.plexus.compiler.AbstractCompiler;
 import org.codehaus.plexus.compiler.CompilerConfiguration;
@@ -102,9 +103,9 @@ public class JavacCompiler extends AbstractCompiler {
 
     private static final String JAVAC_CLASSNAME = "com.sun.tools.javac.Main";
 
-    private static volatile Class<?> JAVAC_CLASS;
+    private volatile Class<?> javacClass;
 
-    private final List<Class<?>> javaccClasses = new CopyOnWriteArrayList<>();
+    private final Deque<Class<?>> javacClasses = new ConcurrentLinkedDeque<>();
 
     @Inject
     private InProcessCompiler inProcessCompiler;
@@ -955,7 +956,7 @@ private static String getJavacExecutable() throws IOException {
     private void releaseJavaccClass(Class<?> javaccClass, CompilerConfiguration compilerConfiguration) {
         if (compilerConfiguration.getCompilerReuseStrategy()
                 == CompilerConfiguration.CompilerReuseStrategy.ReuseCreated) {
-            javaccClasses.add(javaccClass);
+            javacClasses.add(javaccClass);
         }
     }
 
@@ -966,32 +967,28 @@ private void releaseJavaccClass(Class<?> javaccClass, CompilerConfiguration comp
      * @throws CompilerException if the class has not been found.
      */
     private Class<?> getJavacClass(CompilerConfiguration compilerConfiguration) throws CompilerException {
-        Class<?> c = null;
+        Class<?> c;
         switch (compilerConfiguration.getCompilerReuseStrategy()) {
             case AlwaysNew:
                 return createJavacClass();
             case ReuseCreated:
-                synchronized (javaccClasses) {
-                    if (javaccClasses.size() > 0) {
-                        c = javaccClasses.get(0);
-                        javaccClasses.remove(c);
-                        return c;
-                    }
+                c = javacClasses.poll();
+                if (c == null) {
+                    c = createJavacClass();
                 }
-                c = createJavacClass();
                 return c;
             case ReuseSame:
             default:
-                c = JavacCompiler.JAVAC_CLASS;
-                if (c != null) {
-                    return c;
-                }
-                synchronized (JavacCompiler.LOCK) {
-                    if (c == null) {
-                        JavacCompiler.JAVAC_CLASS = c = createJavacClass();
+                c = javacClass;
+                if (c == null) {
+                    synchronized (this) {
+                        c = javacClass;
+                        if (c == null) {
+                            javacClass = c = createJavacClass();
+                        }
                     }
-                    return c;
                 }
+                return c;
         }
     }