wip: add in a sync executor to pass tasks back to the gradle thread in a less brittle way

starts on fixing #39

Author: zml2008

subprojects/gradle-plugin/src/main/java/org/spongepowered/gradle/vanilla/internal/bundler/BundleElement.java

@@ -25,7 +25,6 @@
 package org.spongepowered.gradle.vanilla.internal.bundler;
 
 import org.immutables.value.Value;
-import org.spongepowered.gradle.vanilla.internal.model.GroupArtifactVersion;
 
 /**
  * A single entry in a bundle.

subprojects/gradle-plugin/src/main/java/org/spongepowered/gradle/vanilla/internal/bundler/BundlerMetadata.java

@@ -59,10 +59,12 @@ public abstract class BundlerMetadata {
     /**
      * Attempt to read bundler metadata from a jar.
      *
-     * <p>If the jar is not a Minecraft bundler jar, an empty {@link Optional} will be returned.</p>
+     * <p>If the jar is not a Minecraft bundler jar, an empty {@link Optional} will
+     * be returned.</p>
      *
      * @param jar the jar to read
      * @return parsed metadata
+     * @throws IOException if an error occurs while trying to read from the jar
      */
     public static Optional<BundlerMetadata> read(final Path jar) throws IOException {
         try (final JarFile file = new JarFile(jar)) {

subprojects/gradle-plugin/src/main/java/org/spongepowered/gradle/vanilla/internal/repository/LauncherMetaMetadataSupplierAndArtifactProducer.java

@@ -38,6 +38,7 @@
 import org.spongepowered.gradle.vanilla.resolver.ResolutionResult;
 
 import java.util.Optional;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 
 import javax.inject.Inject;
@@ -74,8 +75,10 @@ public void execute(final ComponentMetadataSupplierDetails details) {
             final MinecraftResolver resolver = providerService.resolver();
             // Request the appropriate jar, block until it's provided
             // TODO: maybe validate that the state keys of the provided modifiers actually match the artifact ID?
-            final ResolutionResult<MinecraftResolver.MinecraftEnvironment>
-                resolution = resolver.provide(platform.get(), version, providerService.peekModifiers()).get();
+            final CompletableFuture<ResolutionResult<MinecraftResolver.MinecraftEnvironment>> resolutionFuture = resolver
+                .provide(platform.get(), version, providerService.peekModifiers());
+
+            final ResolutionResult<MinecraftResolver.MinecraftEnvironment> resolution = resolver.processSyncTasksUntilComplete(resolutionFuture);
             if (!resolution.isPresent()) {
                 return;
             }

subprojects/gradle-plugin/src/main/java/org/spongepowered/gradle/vanilla/internal/repository/MinecraftRepositoryPlugin.java

@@ -42,8 +42,8 @@
 import org.gradle.api.plugins.ExtensionAware;
 import org.gradle.api.provider.Provider;
 import org.gradle.build.event.BuildEventsListenerRegistry;
-import org.spongepowered.gradle.vanilla.internal.Constants;
 import org.spongepowered.gradle.vanilla.MinecraftExtension;
+import org.spongepowered.gradle.vanilla.internal.Constants;
 import org.spongepowered.gradle.vanilla.internal.MinecraftExtensionImpl;
 import org.spongepowered.gradle.vanilla.internal.model.VersionClassifier;
 import org.spongepowered.gradle.vanilla.internal.repository.modifier.ArtifactModifier;
@@ -184,7 +184,7 @@ private void configureResolutionStrategy(
                     // If we do have a version, try to resolve that fixed version
                     if (version != null) {
                         try {
-                            resolver.provide(platform.get(), version, providerService.peekModifiers()).get();
+                            resolver.processSyncTasksUntilComplete(resolver.provide(platform.get(), version, providerService.peekModifiers()));
                         } catch (final InterruptedException ex) {
                             Thread.currentThread().interrupt();
                         } catch (final ExecutionException ex) {

subprojects/gradle-plugin/src/main/java/org/spongepowered/gradle/vanilla/internal/transformer/AtlasTransformers.java

@@ -28,7 +28,6 @@
 import org.cadixdev.bombe.asm.jar.JarEntryRemappingTransformer;
 import org.cadixdev.bombe.jar.JarEntryTransformer;
 import org.cadixdev.lorenz.MappingSet;
-import org.cadixdev.lorenz.asm.LorenzRemapper;
 import org.spongepowered.gradle.vanilla.internal.asm.EnhancedClassRemapper;
 import org.spongepowered.gradle.vanilla.internal.asm.EnhancedRemapper;
 import org.spongepowered.gradle.vanilla.internal.asm.LocalVariableNamingClassVisitor;

subprojects/gradle-plugin/src/main/java/org/spongepowered/gradle/vanilla/repository/MinecraftResolver.java

@@ -27,16 +27,17 @@
 import org.spongepowered.gradle.vanilla.internal.model.GroupArtifactVersion;
 import org.spongepowered.gradle.vanilla.internal.model.VersionDescriptor;
 import org.spongepowered.gradle.vanilla.internal.model.VersionManifestRepository;
-import org.spongepowered.gradle.vanilla.resolver.Downloader;
 import org.spongepowered.gradle.vanilla.internal.repository.ResolvableTool;
 import org.spongepowered.gradle.vanilla.internal.repository.modifier.ArtifactModifier;
 import org.spongepowered.gradle.vanilla.internal.repository.modifier.AssociatedResolutionFlags;
+import org.spongepowered.gradle.vanilla.resolver.Downloader;
 import org.spongepowered.gradle.vanilla.resolver.ResolutionResult;
 
 import java.net.URLClassLoader;
 import java.nio.file.Path;
 import java.util.Set;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executor;
 import java.util.function.BiConsumer;
 import java.util.function.Supplier;
@@ -96,7 +97,21 @@ public interface MinecraftResolver {
      *     environment and a target path
      * @return a future returning the result of resolving a jar path
      */
-    CompletableFuture<ResolutionResult<Path>> produceAssociatedArtifactSync(final MinecraftPlatform side, final String version, final Set<ArtifactModifier> modifiers, final String id, final Set<AssociatedResolutionFlags> flags, final BiConsumer<MinecraftEnvironment, Path> action);
+    CompletableFuture<ResolutionResult<Path>> produceAssociatedArtifactSync(
+        final MinecraftPlatform side, final String version, final Set<ArtifactModifier> modifiers, final String id,
+        final Set<AssociatedResolutionFlags> flags, final BiConsumer<MinecraftEnvironment, Path> action
+    );
+
+    /**
+     * Block on the completion of a provided future, processing "sync" tasks while
+     * that occurs.
+     * 
+     * @param <T> the return value type
+     * @param future the future to await
+     * @return the result of the future
+     * @throws ExecutionException if the task execution fails
+     */
+    <T> T processSyncTasksUntilComplete(CompletableFuture<T> future) throws ExecutionException, InterruptedException;
 
     interface MinecraftEnvironment {
 
@@ -162,13 +177,22 @@ interface Context {
         Executor executor();
 
         /**
-         * Return a child classloader with a tool and its dependencies on the
-         * classpath, as well as the VanillaGradle jar.
+         * An executor for performing main-thread synchronous operations, like some
+         * dependency resolution.
+         * 
+         * @return the synchronous executor
+         */
+        Executor syncExecutor();
+
+        /**
+         * Return a child classloader with a tool and its dependencies on the classpath,
+         * as well as the VanillaGradle jar.
          *
-         * <p>This is a very fragile arrangement but it allows some dependencies
-         * to be overridden at runtime. Classes from Gradle, VanillaGradle's
-         * dependencies, and the JDK can be safely shared, but VanillaGradle
-         * classes CAN NOT.</p>
+         * <p>This is a very fragile arrangement but it allows some dependencies to be
+         * overridden at runtime. Classes from Gradle, VanillaGradle's dependencies, and
+         * the JDK can be safely shared, but VanillaGradle classes CAN NOT.</p>
+         * 
+         * <p>This must be run on the {@link #syncExecutor()}.</p>
          *
          * @param tool the tool to resolve
          * @return a class loader with the tool on the classpath

subprojects/gradle-plugin/src/main/java/org/spongepowered/gradle/vanilla/repository/MinecraftResolverImpl.java

@@ -42,17 +42,17 @@
 import org.spongepowered.gradle.vanilla.internal.model.GroupArtifactVersion;
 import org.spongepowered.gradle.vanilla.internal.model.VersionDescriptor;
 import org.spongepowered.gradle.vanilla.internal.model.VersionManifestRepository;
-import org.spongepowered.gradle.vanilla.internal.util.FunctionalUtils;
-import org.spongepowered.gradle.vanilla.resolver.Downloader;
-import org.spongepowered.gradle.vanilla.resolver.HashAlgorithm;
 import org.spongepowered.gradle.vanilla.internal.repository.IvyModuleWriter;
 import org.spongepowered.gradle.vanilla.internal.repository.ResolvableTool;
 import org.spongepowered.gradle.vanilla.internal.repository.modifier.ArtifactModifier;
 import org.spongepowered.gradle.vanilla.internal.repository.modifier.AssociatedResolutionFlags;
-import org.spongepowered.gradle.vanilla.internal.transformer.AtlasTransformers;
 import org.spongepowered.gradle.vanilla.internal.resolver.AsyncUtils;
 import org.spongepowered.gradle.vanilla.internal.resolver.FileUtils;
+import org.spongepowered.gradle.vanilla.internal.transformer.AtlasTransformers;
+import org.spongepowered.gradle.vanilla.internal.util.FunctionalUtils;
 import org.spongepowered.gradle.vanilla.internal.util.SelfPreferringClassLoader;
+import org.spongepowered.gradle.vanilla.resolver.Downloader;
+import org.spongepowered.gradle.vanilla.resolver.HashAlgorithm;
 import org.spongepowered.gradle.vanilla.resolver.ResolutionResult;
 
 import java.io.IOException;
@@ -71,13 +71,15 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executor;
 import java.util.concurrent.ExecutorService;
+import java.util.concurrent.SynchronousQueue;
 import java.util.function.BiConsumer;
 import java.util.function.Function;
 import java.util.function.Supplier;
@@ -95,6 +97,8 @@ public class MinecraftResolverImpl implements MinecraftResolver, MinecraftResolv
     private final ConcurrentMap<EnvironmentKey, CompletableFuture<ResolutionResult<MinecraftEnvironment>>> artifacts = new ConcurrentHashMap<>();
     private final ConcurrentMap<EnvironmentKey, CompletableFuture<ResolutionResult<Path>>> associatedArtifacts = new ConcurrentHashMap<>();
     private final boolean forceRefresh;
+    private final BlockingQueue<Runnable> syncTasks = new SynchronousQueue<>();
+    private final Executor syncExecutor = run -> this.syncTasks.add(run);
 
     public MinecraftResolverImpl(
         final VersionManifestRepository manifests,
@@ -127,6 +131,11 @@ public Executor executor() {
         return this.executor;
     }
 
+    @Override
+    public Executor syncExecutor() {
+        return this.syncExecutor;
+    }
+
     @Override
     public Supplier<URLClassLoader> classLoaderWithTool(final ResolvableTool tool) {
         final @Nullable Function<ResolvableTool, URL[]> toolResolver = this.toolResolver;
@@ -520,6 +529,32 @@ public CompletableFuture<ResolutionResult<Path>> produceAssociatedArtifactSync(
         return ourResult;
     }
 
+    @Override
+    public <T> T processSyncTasksUntilComplete(CompletableFuture<T> future) throws InterruptedException, ExecutionException {
+        future.handle((res, err) -> {
+            this.syncTasks.add(new CompleteEvaluation(future));
+            return res;
+        });
+
+        Runnable action;
+        for (;;) {
+            action = this.syncTasks.take();
+
+            // todo: rethrow exceptions with an ExecutionException
+            if (action instanceof CompleteEvaluation && ((CompleteEvaluation) action).completed == future) {
+                break;
+            }
+
+            try {
+                action.run();
+            } catch (final Exception ex) {
+                MinecraftResolverImpl.LOGGER.error("Failed to execute synchronous task {} while resolving {}", action, future, ex);
+            }
+        }
+
+        return future.get();
+    }
+
     private String sharedArtifactFileName(
         final String artifactId,
         final String version,
@@ -668,4 +703,19 @@ public VersionDescriptor.Full metadata() {
 
     }
 
+    static final class CompleteEvaluation implements Runnable {
+
+        final CompletableFuture<?> completed;
+
+        CompleteEvaluation(final CompletableFuture<?> completed) {
+            this.completed = completed;
+        }
+
+        @Override
+        public void run() {
+            // no-op
+        }
+
+    }
+
 }

subprojects/gradle-plugin/src/main/java/org/spongepowered/gradle/vanilla/repository/MinecraftSide.java

@@ -26,7 +26,6 @@
 
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.spongepowered.gradle.vanilla.internal.Constants;
-import org.spongepowered.gradle.vanilla.internal.bundler.BundleElement;
 import org.spongepowered.gradle.vanilla.internal.bundler.BundlerMetadata;
 import org.spongepowered.gradle.vanilla.internal.model.DownloadClassifier;
 import org.spongepowered.gradle.vanilla.internal.model.GroupArtifactVersion;

subprojects/gradle-plugin/src/main/java/org/spongepowered/gradle/vanilla/task/DecompileJarTask.java

@@ -43,15 +43,15 @@
 import org.gradle.internal.component.external.model.ModuleComponentArtifactIdentifier;
 import org.gradle.jvm.toolchain.JavaLauncher;
 import org.gradle.workers.WorkerExecutor;
-import org.spongepowered.gradle.vanilla.internal.Constants;
 import org.spongepowered.gradle.vanilla.MinecraftExtension;
+import org.spongepowered.gradle.vanilla.internal.Constants;
 import org.spongepowered.gradle.vanilla.internal.MinecraftExtensionImpl;
-import org.spongepowered.gradle.vanilla.repository.MinecraftPlatform;
 import org.spongepowered.gradle.vanilla.internal.repository.MinecraftProviderService;
-import org.spongepowered.gradle.vanilla.resolver.ResolutionResult;
 import org.spongepowered.gradle.vanilla.internal.repository.modifier.ArtifactModifier;
 import org.spongepowered.gradle.vanilla.internal.repository.modifier.AssociatedResolutionFlags;
 import org.spongepowered.gradle.vanilla.internal.worker.JarDecompileWorker;
+import org.spongepowered.gradle.vanilla.repository.MinecraftPlatform;
+import org.spongepowered.gradle.vanilla.resolver.ResolutionResult;
 
 import java.io.File;
 import java.lang.management.ManagementFactory;
@@ -185,19 +185,20 @@ public void execute() {
                     this.getWorkerExecutor().await();
                 }
             );
-        } finally {
-            DecompileJarTask.DECOMPILE_LOCK.unlock();
-        }
 
         try {
-            final ResolutionResult<Path> result = resultFuture.get();
+            final ResolutionResult<Path> result = minecraftProvider.resolver().processSyncTasksUntilComplete(resultFuture);
             this.setDidWork(!result.upToDate());
         } catch (final ExecutionException ex) {
             throw new GradleException("Failed to decompile " + this.getMinecraftVersion().get(), ex.getCause());
         } catch (final InterruptedException ex) {
             Thread.currentThread().interrupt();
             throw new GradleException("Interrupted");
         }
+
+    } finally {
+        DecompileJarTask.DECOMPILE_LOCK.unlock();
+    }
     }
 
 }

subprojects/gradle-plugin/src/main/java/org/spongepowered/gradle/vanilla/task/GenEclipseRuns.java

@@ -25,7 +25,6 @@
 package org.spongepowered.gradle.vanilla.task;
 
 import org.gradle.api.DefaultTask;
-import org.gradle.api.InvalidUserDataException;
 import org.gradle.api.file.DirectoryProperty;
 import org.gradle.api.provider.Property;
 import org.gradle.api.provider.SetProperty;
@@ -37,7 +36,6 @@
 import org.spongepowered.gradle.vanilla.internal.runs.EclipseRunConfigurationWriter;
 import org.spongepowered.gradle.vanilla.runs.RunConfiguration;
 
-import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;