Merge pull request #51272 from aloubyansky/retry-acquire-lock-errors

Re-try Maven resolver lock acquisition errors with a blocking task runner

Author: aloubyansky

independent-projects/bootstrap/maven-resolver/src/main/java/io/quarkus/bootstrap/resolver/maven/ApplicationDependencyResolver.java

@@ -7,21 +7,17 @@
 import static io.quarkus.bootstrap.util.DependencyUtils.newDependencyBuilder;
 
 import java.io.BufferedReader;
-import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.nio.file.Files;
-import java.nio.file.NoSuchFileException;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
-import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentLinkedDeque;
 import java.util.function.BiConsumer;
@@ -294,8 +290,14 @@ private void injectDeploymentDeps() {
 
     private Collection<AppDep> collectDeploymentDeps() {
         final ConcurrentLinkedDeque<AppDep> injectQueue = new ConcurrentLinkedDeque<>();
-        var taskRunner = deploymentInjectionPoints.size() == 1 ? ModelResolutionTaskRunner.getBlockingTaskRunner()
-                : getTaskRunner();
+        final ModelResolutionTaskRunner taskRunner;
+        if (deploymentInjectionPoints.size() == 1 || BLOCKING_TASK_RUNNER) {
+            taskRunner = ModelResolutionTaskRunner.getBlockingTaskRunner();
+        } else {
+            // We've been running into Maven resolver failures to acquire a lock to a local fail when resolving dependencies lately.
+            // This error handler will catch those errors and will re-try the corresponding tasks with the blocking task runner.
+            taskRunner = ModelResolutionTaskRunner.getNonBlockingTaskRunner(new RetryLockAcquisitionErrorHandler());
+        }
         for (AppDep extDep : deploymentInjectionPoints) {
             extDep.scheduleCollectDeploymentDeps(taskRunner, injectQueue);
         }
@@ -821,72 +823,20 @@ private static Properties readExtensionProperties(PathVisit visit) {
     private DependencyNode collectDependencies(Artifact artifact, Collection<Exclusion> exclusions,
             List<RemoteRepository> repos) {
         final CollectRequest collectRequest = getCollectRequest(artifact, exclusions, repos);
-        DependencyNode root = null;
+        final DependencyNode root;
         try {
             root = resolver.getSystem()
                     .collectDependencies(resolver.getSession(), collectRequest)
                     .getRoot();
         } catch (DependencyCollectionException e) {
-            // It could happen, especially in Maven 3.8, that multiple threads could end up writing/reading
-            // the same temporary files while resolving the same artifact. Once one of the threads completes
-            // resolving the artifact, the temporary file will be renamed to the target artifact file
-            // and the other thread will fail with one of the file-not-found exceptions.
-            // In this case, we simply re-try the collect request, which should now pick up the already resolved artifact.
-            String missingFile = getMissingFileOrNull(e);
-            if (missingFile != null) {
-                Set<String> missingFiles = new HashSet<>();
-                while (missingFile != null) {
-                    if (missingFiles.add(missingFile)) {
-                        log.debugf("Re-trying the collect request for %s due to missing %s", artifact, missingFile);
-                        try {
-                            root = resolver.getSystem()
-                                    .collectDependencies(resolver.getSession(), collectRequest)
-                                    .getRoot();
-                            break;
-                        } catch (DependencyCollectionException dce) {
-                            missingFile = getMissingFileOrNull(dce);
-                        }
-                    } else {
-                        // if it's the second time it's missing, we give up
-                        throw wrapInDeploymentInjectionException(artifact, e);
-                    }
-                }
-            }
-            if (root == null) {
-                throw wrapInDeploymentInjectionException(artifact, e);
-            }
+            throw new DeploymentInjectionException("Failed to collect dependencies for " + artifact, e);
         }
         if (root.getChildren().size() != 1) {
             throw new DeploymentInjectionException("Only one child expected but got " + root.getChildren());
         }
         return root.getChildren().get(0);
     }
 
-    private static DeploymentInjectionException wrapInDeploymentInjectionException(Artifact artifact, Exception e) {
-        return new DeploymentInjectionException("Failed to collect dependencies for " + artifact, e);
-    }
-
-    /**
-     * Checks whether the cause of this exception a kind of no-such-file exception and returns the file that was missing.
-     *
-     * @param dce top level exception
-     * @return missing file that was the cause or null, if the cause was different
-     */
-    private static String getMissingFileOrNull(DependencyCollectionException dce) {
-        Throwable t = dce.getCause();
-        while (t != null) {
-            var cause = t.getCause();
-            // It looks like in Maven 3.9 it's NoSuchFileException, while in Maven 3.8 it's FileNotFoundException
-            if (cause instanceof NoSuchFileException e) {
-                return e.getFile();
-            } else if (cause instanceof FileNotFoundException) {
-                return cause.getMessage();
-            }
-            t = cause;
-        }
-        return null;
-    }
-
     private CollectRequest getCollectRequest(Artifact artifact, Collection<Exclusion> exclusions,
             List<RemoteRepository> repos) {
         final ArtifactDescriptorResult descr;

independent-projects/bootstrap/maven-resolver/src/main/java/io/quarkus/bootstrap/resolver/maven/BlockingModelResolutionTaskRunner.java

@@ -8,15 +8,36 @@
 class BlockingModelResolutionTaskRunner implements ModelResolutionTaskRunner {
 
     static BlockingModelResolutionTaskRunner getInstance() {
-        return new BlockingModelResolutionTaskRunner();
+        return new BlockingModelResolutionTaskRunner(null);
+    }
+
+    static BlockingModelResolutionTaskRunner getInstance(ModelResolutionTaskErrorHandler errorHandler) {
+        return new BlockingModelResolutionTaskRunner(errorHandler);
+    }
+
+    private final ModelResolutionTaskErrorHandler errorHandler;
+
+    BlockingModelResolutionTaskRunner(ModelResolutionTaskErrorHandler errorHandler) {
+        this.errorHandler = errorHandler;
     }
 
     @Override
     public void run(ModelResolutionTask task) {
-        task.run();
+        if (errorHandler == null) {
+            task.run();
+            return;
+        }
+        try {
+            task.run();
+        } catch (Exception e) {
+            errorHandler.handleError(task, e);
+        }
     }
 
     @Override
     public void waitForCompletion() {
+        if (errorHandler != null) {
+            errorHandler.allTasksFinished();
+        }
     }
 }

independent-projects/bootstrap/maven-resolver/src/main/java/io/quarkus/bootstrap/resolver/maven/FailAtCompletionErrorHandler.java

@@ -0,0 +1,64 @@
+package io.quarkus.bootstrap.resolver.maven;
+
+import java.util.Collection;
+import java.util.concurrent.ConcurrentLinkedDeque;
+
+import org.jboss.logging.Logger;
+
+/**
+ * Error handler that collects all the task execution errors and throws a single one reporting all the captured
+ * failures once all the tasks have finished.
+ */
+public class FailAtCompletionErrorHandler implements ModelResolutionTaskErrorHandler {
+
+    private static final Logger log = Logger.getLogger(FailAtCompletionErrorHandler.class);
+
+    /**
+     * Errors caught while running tasks
+     */
+    private final Collection<Exception> errors = new ConcurrentLinkedDeque<>();
+
+    @Override
+    public void handleError(ModelResolutionTask task, Exception error) {
+        errors.add(error);
+    }
+
+    @Override
+    public void allTasksFinished() {
+        if (!errors.isEmpty()) {
+            throwFailureReport(errors);
+        }
+    }
+
+    protected boolean isEmpty() {
+        return errors.isEmpty();
+    }
+
+    private static void throwFailureReport(Collection<Exception> errors) {
+        var sb = new StringBuilder(
+                "The following errors were encountered while processing Quarkus application dependencies:");
+        log.error(sb);
+        var i = 1;
+        for (var error : errors) {
+            var prefix = i++ + ")";
+            log.error(prefix, error);
+            sb.append(System.lineSeparator()).append(prefix).append(" ").append(error.getLocalizedMessage());
+            for (var e : error.getStackTrace()) {
+                sb.append(System.lineSeparator());
+                for (int j = 0; j < prefix.length(); ++j) {
+                    sb.append(" ");
+                }
+                sb.append("at ").append(e);
+                if (e.getClassName().contains("io.quarkus")) {
+                    sb.append(System.lineSeparator());
+                    for (int j = 0; j < prefix.length(); ++j) {
+                        sb.append(" ");
+                    }
+                    sb.append("...");
+                    break;
+                }
+            }
+        }
+        throw new RuntimeException(sb.toString());
+    }
+}

independent-projects/bootstrap/maven-resolver/src/main/java/io/quarkus/bootstrap/resolver/maven/ModelResolutionTaskErrorHandler.java

@@ -0,0 +1,9 @@
+package io.quarkus.bootstrap.resolver.maven;
+
+public interface ModelResolutionTaskErrorHandler {
+
+    void handleError(ModelResolutionTask task, Exception error);
+
+    default void allTasksFinished() {
+    }
+}

independent-projects/bootstrap/maven-resolver/src/main/java/io/quarkus/bootstrap/resolver/maven/ModelResolutionTaskRunner.java

@@ -6,23 +6,45 @@
 public interface ModelResolutionTaskRunner {
 
     /**
-     * Returns an instance of a non-blocking task runner.
+     * Returns an instance of a non-blocking task runner with an error handler
+     * that collects all task execution errors and throws a single error when all the tasks
+     * have finished.
      *
      * @return an instance of a non-blocking task runner
      */
     static ModelResolutionTaskRunner getNonBlockingTaskRunner() {
-        return new NonBlockingModelResolutionTaskRunner();
+        return getNonBlockingTaskRunner(new FailAtCompletionErrorHandler());
     }
 
     /**
-     * Returns an instance of a blocking task runner.
+     * Returns an instance of a non-blocking task runner with a custom error handler.
+     *
+     * @param errorHandler error handler
+     * @return an instance of non-blocking task runner
+     */
+    static ModelResolutionTaskRunner getNonBlockingTaskRunner(ModelResolutionTaskErrorHandler errorHandler) {
+        return new NonBlockingModelResolutionTaskRunner(errorHandler);
+    }
+
+    /**
+     * Returns an instance of a blocking task runner with no error handler.
+     * Exceptions thrown from tasks will be immediately propagated to the caller.
      *
      * @return an instance of a blocking task runner
      */
     static ModelResolutionTaskRunner getBlockingTaskRunner() {
         return BlockingModelResolutionTaskRunner.getInstance();
     }
 
+    /**
+     * Returns an instance of a blocking task runner with a custom error handler.
+     *
+     * @return an instance of a blocking task runner
+     */
+    static ModelResolutionTaskRunner getBlockingTaskRunner(ModelResolutionTaskErrorHandler errorHandler) {
+        return BlockingModelResolutionTaskRunner.getInstance(errorHandler);
+    }
+
     /**
      * Instructs a runner to run the passed in task.
      * <p>

independent-projects/bootstrap/maven-resolver/src/main/java/io/quarkus/bootstrap/resolver/maven/NonBlockingModelResolutionTaskRunner.java

@@ -1,25 +1,21 @@
 package io.quarkus.bootstrap.resolver.maven;
 
-import java.util.Collection;
+import java.util.Objects;
 import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ConcurrentLinkedDeque;
 import java.util.concurrent.Phaser;
 
-import org.jboss.logging.Logger;
-
 /**
  * Non-blocking task runner implementation
  */
 class NonBlockingModelResolutionTaskRunner implements ModelResolutionTaskRunner {
 
-    private static final Logger log = Logger.getLogger(NonBlockingModelResolutionTaskRunner.class);
-
     private final Phaser phaser = new Phaser(1);
 
-    /**
-     * Errors caught while running tasks
-     */
-    private final Collection<Exception> errors = new ConcurrentLinkedDeque<>();
+    private final ModelResolutionTaskErrorHandler errorHandler;
+
+    NonBlockingModelResolutionTaskRunner(ModelResolutionTaskErrorHandler errorHandler) {
+        this.errorHandler = Objects.requireNonNull(errorHandler, "errorHandler cannot be null");
+    }
 
     /**
      * Runs a model resolution task asynchronously. This method may return before the task has completed.
@@ -33,7 +29,7 @@ public void run(ModelResolutionTask task) {
             try {
                 task.run();
             } catch (Exception e) {
-                errors.add(e);
+                errorHandler.handleError(task, e);
             } finally {
                 phaser.arriveAndDeregister();
             }
@@ -49,36 +45,6 @@ public void run(ModelResolutionTask task) {
     @Override
     public void waitForCompletion() {
         phaser.arriveAndAwaitAdvance();
-        assertNoErrors();
-    }
-
-    private void assertNoErrors() {
-        if (!errors.isEmpty()) {
-            var sb = new StringBuilder(
-                    "The following errors were encountered while processing Quarkus application dependencies:");
-            log.error(sb);
-            var i = 1;
-            for (var error : errors) {
-                var prefix = i++ + ")";
-                log.error(prefix, error);
-                sb.append(System.lineSeparator()).append(prefix).append(" ").append(error.getLocalizedMessage());
-                for (var e : error.getStackTrace()) {
-                    sb.append(System.lineSeparator());
-                    for (int j = 0; j < prefix.length(); ++j) {
-                        sb.append(" ");
-                    }
-                    sb.append("at ").append(e);
-                    if (e.getClassName().contains("io.quarkus")) {
-                        sb.append(System.lineSeparator());
-                        for (int j = 0; j < prefix.length(); ++j) {
-                            sb.append(" ");
-                        }
-                        sb.append("...");
-                        break;
-                    }
-                }
-            }
-            throw new RuntimeException(sb.toString());
-        }
+        errorHandler.allTasksFinished();
     }
 }