fix: apply all 8 concurrency and resource management fixes

Fixes #366 #367 #368 #369 #370 #371 #372 #373

- Add ReadWriteLock to MavenNexusClassSource and MavenRepositoryClassSource
- Replace ArrayList with CopyOnWriteArrayList for thread-safe iteration
- Fix readFully() silent data corruption in MavenRepositoryClassSource
- Remove perArtifactLocks.clear() to prevent lock map race
- Add size limits to NexusClassSource fetchUrl() and loadClassFromJar()
- Add concurrent modification prevention tests

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Author: Flossy

src/main/java/org/flossware/classloader/MavenNexusClassSource$1.class

@@ -21,6 +21,7 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.FutureTask;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 
@@ -52,6 +53,7 @@ public class MavenNexusClassSource implements ClassSource, AutoCloseable {
     private final int connectTimeout;
     private final int readTimeout;
     private volatile boolean closed = false;
+    private final ReentrantReadWriteLock closeLock = new ReentrantReadWriteLock();
 
     /**
      * Creates a Maven Nexus class source with full configuration including timeouts.
@@ -122,43 +124,58 @@ public MavenNexusClassSource(String nexusUrl, String repository, List<MavenArtif
      */
     @Override
     public byte[] loadClassData(String className) throws IOException {
-        if (closed) {
-            throw new IllegalStateException("MavenNexusClassSource is closed");
-        }
-
         Objects.requireNonNull(className, "className cannot be null");
+
+        // Check cache first without locking (optimization for cache hits)
         String cacheKey = className;
-        // Atomic get() - avoids TOCTOU race condition with contains() + get()
         byte[] cachedData = classCache.get(cacheKey);
         if (cachedData != null) {
             return cachedData;
         }
 
-        String classFileName = ClassNameUtil.toClassFilePath(className);
-        List<String> errorMessages = new ArrayList<>();
+        // Acquire read lock to prevent close() from running while we create resources.
+        // Multiple loadClassData() calls can proceed concurrently (read lock is shared),
+        // but close() acquires the write lock (exclusive) and must wait for all loads to finish.
+        closeLock.readLock().lock();
+        try {
+            if (closed) {
+                throw new IllegalStateException("MavenNexusClassSource is closed");
+            }
 
-        for (MavenArtifact artifact : artifacts) {
-            try {
-                String jarUrl = buildJarUrl(artifact);
-                String artifactKey = artifact.toString();
-                JarFile jarFile = ensureJarCached(artifactKey, jarUrl);
-                byte[] classData = extractClassFromCachedJar(jarFile, classFileName, jarUrl);
-                classCache.put(cacheKey, classData);
-                return classData;
-            } catch (IOException e) {
-                // Accumulate errors instead of silently swallowing
-                String errorMsg = String.format("Artifact %s - %s",
-                    artifact.toString(), e.getMessage());
-                errorMessages.add(errorMsg);
+            // Double-check cache (another thread may have loaded it while we waited for lock)
+            cachedData = classCache.get(cacheKey);
+            if (cachedData != null) {
+                return cachedData;
+            }
+
+            String classFileName = ClassNameUtil.toClassFilePath(className);
+            List<String> errorMessages = new ArrayList<>();
+
+            for (MavenArtifact artifact : artifacts) {
+                try {
+                    String jarUrl = buildJarUrl(artifact);
+                    String artifactKey = artifact.toString();
+                    JarFile jarFile = ensureJarCached(artifactKey, jarUrl);
+                    byte[] classData = extractClassFromCachedJar(jarFile, classFileName, jarUrl);
+                    classCache.put(cacheKey, classData);
+                    return classData;
+                } catch (IOException e) {
+                    // Accumulate errors instead of silently swallowing
+                    String errorMsg = String.format("Artifact %s - %s",
+                        artifact.toString(), e.getMessage());
+                    errorMessages.add(errorMsg);
+                }
             }
-        }
 
-        // Throw with ALL error details
-        String allErrors = String.join("\n  - ", errorMessages);
-        throw new IOException(
-            "Class not found in any of " + artifacts.size() + " configured Maven artifacts: " +
-            className + "\nAttempted artifacts:\n  - " + allErrors
-        );
+            // Throw with ALL error details
+            String allErrors = String.join("\n  - ", errorMessages);
+            throw new IOException(
+                "Class not found in any of " + artifacts.size() + " configured Maven artifacts: " +
+                className + "\nAttempted artifacts:\n  - " + allErrors
+            );
+        } finally {
+            closeLock.readLock().unlock();
+        }
     }
 
     /** {@inheritDoc} */
@@ -176,8 +193,13 @@ public boolean canLoad(String className) {
     /** {@inheritDoc} */
     @Override
     public String getDescription() {
-        return "MavenNexusClassSource[" + nexusUrl + ", repo=" + repository +
-               ", artifacts=" + artifacts.size() + ", auth=" + authConfig.getAuthType() + "]";
+        closeLock.readLock().lock();
+        try {
+            return "MavenNexusClassSource[" + nexusUrl + ", repo=" + repository +
+                   ", artifacts=" + artifacts.size() + ", auth=" + authConfig.getAuthType() + "]";
+        } finally {
+            closeLock.readLock().unlock();
+        }
     }
 
     private String buildJarUrl(MavenArtifact artifact) {
@@ -375,13 +397,23 @@ private void configureAuthentication(HttpURLConnection connection) {
 
     /**
      * Adds a Maven artifact to load classes from.
+     * Thread-safe: synchronizes with {@link #loadClassData(String)} to prevent ConcurrentModificationException.
      *
      * @param artifact The Maven artifact to add
      * @throws NullPointerException if artifact is null
+     * @throws IllegalStateException if this class source is closed
      */
     public void addArtifact(MavenArtifact artifact) {
         Objects.requireNonNull(artifact, "artifact cannot be null");
-        artifacts.add(artifact);
+        closeLock.writeLock().lock();
+        try {
+            if (closed) {
+                throw new IllegalStateException("MavenNexusClassSource is closed");
+            }
+            artifacts.add(artifact);
+        } finally {
+            closeLock.writeLock().unlock();
+        }
     }
 
     /**
@@ -396,11 +428,17 @@ public void addArtifact(String coordinates) {
 
     /**
      * Gets the list of configured Maven artifacts.
+     * Thread-safe: returns a copy of the artifacts list under synchronization.
      *
      * @return a copy of the artifacts list
      */
     public List<MavenArtifact> getArtifacts() {
-        return new ArrayList<>(artifacts);
+        closeLock.readLock().lock();
+        try {
+            return new ArrayList<>(artifacts);
+        } finally {
+            closeLock.readLock().unlock();
+        }
     }
 
     /**
@@ -441,7 +479,11 @@ public void close() throws IOException {
             return;
         }
 
-        synchronized (this) {
+        // Acquire write lock exclusively -- this waits for all in-progress loadClassData()
+        // calls (which hold the read lock) to complete before proceeding, ensuring no new
+        // resources are created after close() sets the closed flag.
+        closeLock.writeLock().lock();
+        try {
             if (closed) {
                 return;
             }
@@ -482,6 +524,8 @@ public void close() throws IOException {
                 exceptions.forEach(ex::addSuppressed);
                 throw ex;
             }
+        } finally {
+            closeLock.writeLock().unlock();
         }
     }
 

src/main/java/org/flossware/classloader/MavenNexusClassSource$Builder.class

@@ -17,6 +17,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 
@@ -29,8 +30,9 @@
  * <p><b>Caching Strategy:</b> JAR files are downloaded once per artifact and cached in temporary files.
  * Multiple class requests from the same artifact reuse the cached JAR without re-downloading.</p>
  *
- * <p><b>Thread Safety:</b> This class is thread-safe. The internal caches use ConcurrentHashMap
- * to support concurrent class loading operations.</p>
+ * <p><b>Thread Safety:</b> This class is thread-safe. The internal caches use ConcurrentHashMap,
+ * and the artifacts collection uses CopyOnWriteArrayList to support safe concurrent modification
+ * and iteration during class loading operations.</p>
  */
 public class MavenRepositoryClassSource implements ClassSource, AutoCloseable {
     private static final long MAX_JAR_SIZE = 100 * 1024 * 1024; // 100MB default max JAR size
@@ -76,7 +78,7 @@ public MavenRepositoryClassSource(String repositoryUrl, List<MavenArtifact> arti
         }
 
         this.repositoryUrl = repositoryUrl.endsWith("/") ? repositoryUrl : repositoryUrl + "/";
-        this.artifacts = new ArrayList<>(artifacts);
+        this.artifacts = new CopyOnWriteArrayList<>(artifacts);
         this.authConfig = authConfig != null ? authConfig : AuthConfig.none();
         this.classCache = new ConcurrentHashMap<>();
         this.jarFileCache = new ConcurrentHashMap<>();
@@ -112,8 +114,10 @@ public MavenRepositoryClassSource(String repositoryUrl, List<MavenArtifact> arti
      * <p>Searches through all configured Maven artifacts in order, downloading JARs
      * and extracting the requested class file. Results are cached in memory.</p>
      *
-     * <p><b>Thread Safety:</b> Uses fine-grained per-artifact synchronization to allow
-     * concurrent downloads of different artifacts while maintaining cache consistency.</p>
+     * <p><b>Thread Safety:</b> Uses CopyOnWriteArrayList for the artifacts collection to ensure
+     * thread-safe iteration even when artifacts are added via addArtifact(). Uses fine-grained
+     * per-artifact synchronization to allow concurrent downloads of different artifacts while
+     * maintaining cache consistency.</p>
      */
     @Override
     public byte[] loadClassData(String className) throws IOException {
@@ -133,6 +137,7 @@ public byte[] loadClassData(String className) throws IOException {
         String classFileName = ClassNameUtil.toClassFilePath(className);
         List<String> errorMessages = new ArrayList<>();
 
+        // artifacts is CopyOnWriteArrayList - thread-safe for iteration during mutation
         for (MavenArtifact artifact : artifacts) {
             try {
                 String jarUrl = buildJarUrl(artifact);
@@ -200,6 +205,12 @@ private JarFile ensureJarCached(String artifactKey, String jarUrl) throws IOExce
         Object artifactLock = perArtifactLocks.computeIfAbsent(artifactKey, k -> new Object());
 
         synchronized (artifactLock) {
+            // Re-check closed flag after acquiring lock - close() may have run
+            // while this thread was waiting, cleaning up all cached resources.
+            if (closed) {
+                throw new IllegalStateException("MavenRepositoryClassSource is closed");
+            }
+
             // Double-check pattern: another thread may have cached it while waiting for lock
             existing = jarFileCache.get(artifactKey);
             if (existing != null) {
@@ -304,7 +315,10 @@ private void readFully(InputStream in, byte[] data, int size) throws IOException
         while (totalRead < size) {
             int n = in.read(data, totalRead, size - totalRead);
             if (n == -1) {
-                return;
+                throw new IOException(
+                    "Unexpected end of stream: expected " + size +
+                    " bytes but only read " + totalRead
+                );
             }
             totalRead += n;
         }
@@ -416,7 +430,13 @@ public void close() throws IOException {
                 }
             }
             jarPathCache.clear();
-            perArtifactLocks.clear();
+            // Note: perArtifactLocks is intentionally NOT cleared. The lock objects
+            // are plain Object instances that hold no resources. Clearing them while
+            // other threads may still be synchronized on them creates a race condition:
+            // a new computeIfAbsent call would create a different lock object for the
+            // same artifact key, allowing two threads into the critical section in
+            // ensureJarCached() concurrently. The locks will be garbage-collected when
+            // this MavenRepositoryClassSource instance is collected.
 
             // Throw aggregated exception if any occurred
             if (!exceptions.isEmpty()) {

src/main/java/org/flossware/classloader/MavenNexusClassSource.class

@@ -24,6 +24,8 @@
  * ConcurrentHashMap to support concurrent class loading operations.</p>
  */
 public class NexusClassSource implements ClassSource {
+    private static final long MAX_CLASS_SIZE = 10 * 1024 * 1024; // 10MB default max class size
+
     private final String nexusUrl;
     private final String repository;
     private final AuthConfig authConfig;
@@ -180,7 +182,14 @@ private byte[] fetchUrl(String urlString) throws IOException {
 
                 byte[] buffer = new byte[DEFAULT_BUFFER_SIZE];
                 int bytesRead;
+                long totalBytes = 0;
+
                 while ((bytesRead = in.read(buffer)) != -1) {
+                    totalBytes += bytesRead;
+                    if (totalBytes > MAX_CLASS_SIZE) {
+                        throw new IOException("Class file too large: " + totalBytes +
+                                            " bytes (max: " + MAX_CLASS_SIZE + " bytes) for URL: " + urlString);
+                    }
                     out.write(buffer, 0, bytesRead);
                 }
 
@@ -219,7 +228,14 @@ protected byte[] loadClassFromJar(String jarUrl, String classFileName) throws IO
                         try (ByteArrayOutputStream out = new ByteArrayOutputStream()) {
                             byte[] buffer = new byte[DEFAULT_BUFFER_SIZE];
                             int bytesRead;
+                            long totalBytes = 0;
+
                             while ((bytesRead = jarIn.read(buffer)) != -1) {
+                                totalBytes += bytesRead;
+                                if (totalBytes > MAX_CLASS_SIZE) {
+                                    throw new IOException("Class file too large: " + totalBytes +
+                                                        " bytes (max: " + MAX_CLASS_SIZE + " bytes) for " + classFileName);
+                                }
                                 out.write(buffer, 0, bytesRead);
                             }
                             return out.toByteArray();

src/main/java/org/flossware/classloader/MavenNexusClassSource.java

@@ -113,4 +113,54 @@ void testAddArtifactAfterCreation() throws Exception {
             assertEquals(2, source.getArtifacts().size());
         }
     }
+
+    @Test
+    void testConcurrentModificationPrevention() throws Exception {
+        MavenArtifact artifact = new MavenArtifact("org.example", "my-lib", "1.0.0");
+        try (MavenNexusClassSource source = new MavenNexusClassSource(
+            "https://nexus.example.com",
+            "releases",
+            Arrays.asList(artifact)
+        )) {
+
+            // Test that concurrent mutations and reads don't cause ConcurrentModificationException
+            Thread mutatorThread = new Thread(() -> {
+                for (int i = 0; i < 100; i++) {
+                    source.addArtifact("org.example:lib-" + i + ":1.0.0");
+                }
+            });
+
+            Thread readerThread = new Thread(() -> {
+                for (int i = 0; i < 100; i++) {
+                    source.getArtifacts();
+                    source.getDescription();
+                }
+            });
+
+            mutatorThread.start();
+            readerThread.start();
+
+            mutatorThread.join();
+            readerThread.join();
+
+            // Verify final count (1 original + 100 added)
+            assertEquals(101, source.getArtifacts().size());
+        }
+    }
+
+    @Test
+    void testAddArtifactThrowsWhenClosed() throws Exception {
+        MavenArtifact artifact = new MavenArtifact("org.example", "my-lib", "1.0.0");
+        MavenNexusClassSource source = new MavenNexusClassSource(
+            "https://nexus.example.com",
+            "releases",
+            Arrays.asList(artifact)
+        );
+
+        source.close();
+
+        assertThrows(IllegalStateException.class, () -> {
+            source.addArtifact("org.example:another-lib:2.0.0");
+        });
+    }
 }