Task/149 remove module class loaders (#168)

* Remove classloaders from modules

* Tidy up container group implementations and classloaders a little

Author: ascopes

java-compiler-testing/src/main/java/io/github/ascopes/jct/assertions/AbstractContainerGroupAssert.java

@@ -52,15 +52,6 @@ protected AbstractContainerGroupAssert(C containerGroup, Class<?> selfType) {
     super(containerGroup, selfType);
   }
 
-  /**
-   * Get assertions to perform on the class loader associated with this container group.
-   *
-   * @return the assertions to perform.
-   */
-  public ClassLoaderAssert classLoader() {
-    return new ClassLoaderAssert(actual.getClassLoader());
-  }
-
   /**
    * Get assertions to perform on the location of this container group.
    *

java-compiler-testing/src/main/java/io/github/ascopes/jct/assertions/PackageContainerGroupAssert.java

@@ -58,7 +58,7 @@ public PackageContainerGroupAssert(PackageContainerGroup containerGroup) {
    * @throws AssertionError if the file exists.
    */
   public PackageContainerGroupAssert fileDoesNotExist(String path) {
-    var file = actual.findFile(path);
+    var file = actual.getFile(path);
 
     if (file == null) {
       return this;
@@ -95,6 +95,15 @@ public void allFilesExist(Iterable<String> paths) {
     assertThat(paths).allSatisfy(this::fileExists);
   }
 
+  /**
+   * Get assertions to perform on the class loader associated with this container group.
+   *
+   * @return the assertions to perform.
+   */
+  public ClassLoaderAssert classLoader() {
+    return new ClassLoaderAssert(actual.getClassLoader());
+  }
+
   /**
    * Assert that the given file exists.
    *
@@ -103,7 +112,7 @@ public void allFilesExist(Iterable<String> paths) {
    * @throws AssertionError if the file does not exist.
    */
   public AbstractPathAssert<?> fileExists(String path) {
-    var file = actual.findFile(path);
+    var file = actual.getFile(path);
 
     if (file != null) {
       return assertThat(file);

java-compiler-testing/src/main/java/io/github/ascopes/jct/compilers/impl/JctFileManagerImpl.java

@@ -28,7 +28,6 @@
 import io.github.ascopes.jct.containers.impl.OutputContainerGroupImpl;
 import io.github.ascopes.jct.containers.impl.PackageContainerGroupImpl;
 import io.github.ascopes.jct.pathwrappers.PathWrapper;
-import io.github.ascopes.jct.utils.GarbageDisposalUtils;
 import io.github.ascopes.jct.utils.ToStringBuilder;
 import java.io.IOException;
 import java.lang.module.ModuleFinder;

java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/ContainerGroup.java

@@ -42,16 +42,6 @@ public interface ContainerGroup extends Closeable {
    */
   boolean contains(PathFileObject fileObject);
 
-  /**
-   * Get a class loader for this group of containers.
-   *
-   * <p>Note that adding additional containers to this group after accessing this class loader
-   * may result in the class loader being destroyed or re-created.
-   *
-   * @return the class loader.
-   */
-  ClassLoader getClassLoader();
-
   /**
    * Get the location of this container group.
    *

java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/OutputContainerGroup.java

@@ -29,8 +29,7 @@
  * @since 0.0.1
  */
 @API(since = "0.0.1", status = Status.EXPERIMENTAL)
-public interface OutputContainerGroup
-    extends PackageContainerGroup, ModuleContainerGroup {
+public interface OutputContainerGroup extends PackageContainerGroup, ModuleContainerGroup {
 
   /**
    * Get the output-oriented location.

java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/PackageContainerGroup.java

@@ -66,6 +66,16 @@ public interface PackageContainerGroup extends ContainerGroup {
    */
   void addPackage(@WillNotClose PathWrapper path);
 
+  /**
+   * Get a class loader for this group of containers.
+   *
+   * <p>Note that adding additional containers to this group after accessing this class loader
+   * may result in the class loader being destroyed or re-created.
+   *
+   * @return the class loader.
+   */
+  ClassLoader getClassLoader();
+
   /**
    * Find the first occurrence of a given path to a file in packages or modules.
    *
@@ -75,7 +85,7 @@ public interface PackageContainerGroup extends ContainerGroup {
    * @return the first occurrence of the path in this group, or null if not found.
    */
   @Nullable
-  Path findFile(String path);
+  Path getFile(String path);
 
   /**
    * Get a {@link FileObject} that can have content read from it.

java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/AbstractPackageContainerGroup.java

@@ -15,6 +15,7 @@
  */
 package io.github.ascopes.jct.containers.impl;
 
+import static java.util.Collections.synchronizedSet;
 import static java.util.Objects.requireNonNull;
 
 import io.github.ascopes.jct.compilers.PathFileObject;
@@ -67,7 +68,7 @@ public abstract class AbstractPackageContainerGroup implements PackageContainerG
 
   // Use a linked hash set here to deduplicate while retaining order. This is an optimisation
   // since defining the same path more than once does not make sense anyway.
-  protected final LinkedHashSet<Container> containers;
+  protected final Set<Container> containers;
   protected final Lazy<ClassLoader> classLoaderLazy;
 
   /**
@@ -80,7 +81,7 @@ protected AbstractPackageContainerGroup(Location location, String release) {
     this.location = requireNonNull(location, "location");
     this.release = requireNonNull(release, "release");
 
-    containers = new LinkedHashSet<>();
+    containers = synchronizedSet(new LinkedHashSet<>());
     classLoaderLazy = new Lazy<>(this::createClassLoader);
   }
 
@@ -149,7 +150,7 @@ public void close() throws IOException {
 
   @Override
   @Nullable
-  public Path findFile(String path) {
+  public Path getFile(String path) {
     for (var container : containers) {
       var result = container.getFile(path);
       if (result != null) {

java-compiler-testing/src/main/java/io/github/ascopes/jct/containers/impl/ContainerGroupUrlClassLoader.java

@@ -23,18 +23,18 @@
 import io.github.ascopes.jct.containers.ModuleContainerGroup;
 import io.github.ascopes.jct.containers.PackageContainerGroup;
 import io.github.ascopes.jct.pathwrappers.PathWrapper;
-import io.github.ascopes.jct.utils.Lazy;
 import io.github.ascopes.jct.utils.StringUtils;
 import io.github.ascopes.jct.utils.ToStringBuilder;
 import java.io.IOException;
 import java.lang.module.ModuleFinder;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.ServiceLoader;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import javax.annotation.Nullable;
 import javax.annotation.WillCloseWhenClosed;
 import javax.tools.JavaFileManager.Location;
 import org.apiguardian.api.API;
@@ -52,7 +52,6 @@ public final class ModuleContainerGroupImpl implements ModuleContainerGroup {
   private final Location location;
   private final Map<ModuleLocation, ModulePackageContainerGroupImpl> modules;
   private final String release;
-  private final Lazy<ClassLoader> classLoaderLazy;
 
   /**
    * Initialize this container group.
@@ -82,8 +81,7 @@ public ModuleContainerGroupImpl(Location location, String release) {
       );
     }
 
-    modules = new HashMap<>();
-    classLoaderLazy = new Lazy<>(this::createClassLoader);
+    modules = new ConcurrentHashMap<>();
   }
 
   @Override
@@ -96,21 +94,6 @@ public void addModule(String module, PathWrapper path) {
     getOrCreateModule(module).addPackage(path);
   }
 
-  @Override
-  public PackageContainerGroup getModule(String module) {
-    if (module.isEmpty()) {
-      throw new IllegalArgumentException("Cannot have module sources with no valid module name");
-    }
-
-    return modules
-        .keySet()
-        .stream()
-        .filter(location -> location.getModuleName().equals(module))
-        .findFirst()
-        .map(modules::get)
-        .orElse(null);
-  }
-
   @Override
   public void close() throws IOException {
     var exceptions = new ArrayList<IOException>();
@@ -143,11 +126,6 @@ public boolean contains(PathFileObject fileObject) {
     return false;
   }
 
-  @Override
-  public ClassLoader getClassLoader() {
-    return classLoaderLazy.access();
-  }
-
   @Override
   public Location getLocation() {
     return location;
@@ -158,14 +136,24 @@ public List<Set<Location>> getLocationsForModules() {
     return List.of(Set.copyOf(modules.keySet()));
   }
 
+  @Nullable
+  @Override
+  public PackageContainerGroup getModule(String name) {
+    if (name.isEmpty()) {
+      throw new IllegalArgumentException("Cannot have module sources with no valid module name");
+    }
+
+    return modules.get(new ModuleLocation(location, name));
+  }
+
   @Override
   public Map<ModuleLocation, PackageContainerGroup> getModules() {
     return Map.copyOf(modules);
   }
 
   @Override
-  public boolean hasLocation(ModuleLocation location) {
-    return modules.containsKey(location);
+  public PackageContainerGroup getOrCreateModule(String moduleName) {
+    return modules.computeIfAbsent(new ModuleLocation(location, moduleName), this::newPackageGroup);
   }
 
   @Override
@@ -193,8 +181,8 @@ public <S> ServiceLoader<S> getServiceLoader(Class<S> service) {
   }
 
   @Override
-  public PackageContainerGroup getOrCreateModule(String moduleName) {
-    return modules.computeIfAbsent(new ModuleLocation(location, moduleName), this::newPackageGroup);
+  public boolean hasLocation(ModuleLocation location) {
+    return modules.containsKey(location);
   }
 
   @Override
@@ -205,10 +193,6 @@ public String toString() {
         .toString();
   }
 
-  private ClassLoader createClassLoader() {
-    return ContainerGroupUrlClassLoader.createClassLoaderFor(this);
-  }
-
   private ModulePackageContainerGroupImpl newPackageGroup(ModuleLocation location) {
     return new ModulePackageContainerGroupImpl(location, release);
   }
@@ -226,7 +210,7 @@ private ModulePackageContainerGroupImpl(ModuleLocation location, String release)
 
     @Override
     protected ClassLoader createClassLoader() {
-      return ContainerGroupUrlClassLoader.createClassLoaderFor(this);
+      return new PackageContainerGroupUrlClassLoader(this);
     }
   }
 }