Fix locking issues (#1660)

Changes:
* introduce dedicated ex on timeout
* up default timeouts (from 30s to 900s)
* improve message by listing all lock subjects
* improve message by mentioning the property that user should use to increase timeouts (there is no one size fits all; we could go with "infinite" timeouts. but am unsure about that)

Author: cstamas

maven-resolver-api/src/main/java/org/eclipse/aether/SyncContext.java

@@ -61,12 +61,39 @@ public interface SyncContext extends Closeable {
      *
      * @param artifacts The artifacts to acquire, may be {@code null} or empty if none.
      * @param metadatas The metadatas to acquire, may be {@code null} or empty if none.
+     * @throws FailedToAcquireLockException if method calls to acquire lock within configured time.
      */
-    void acquire(Collection<? extends Artifact> artifacts, Collection<? extends Metadata> metadatas);
+    void acquire(Collection<? extends Artifact> artifacts, Collection<? extends Metadata> metadatas)
+            throws FailedToAcquireLockException;
 
     /**
      * Releases all previously acquired artifacts/metadatas. If no resources have been acquired before or if this
      * synchronization context has already been closed, this method does nothing.
      */
+    @Override
     void close();
+
+    /**
+     * Specific exception thrown by {@link #acquire(Collection, Collection)} method when it cannot acquire the lock.
+     *
+     * @since 1.9.25
+     */
+    final class FailedToAcquireLockException extends IllegalStateException {
+        private final boolean shared;
+
+        /**
+         * Constructor.
+         */
+        public FailedToAcquireLockException(boolean shared, String message) {
+            super(message);
+            this.shared = shared;
+        }
+
+        /**
+         * Returns {@code true} for shared and {@code false} for exclusive sync contexts.
+         */
+        public boolean isShared() {
+            return shared;
+        }
+    }
 }

maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java

@@ -22,6 +22,7 @@
 import java.util.Collection;
 import java.util.Deque;
 import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
 
 import org.eclipse.aether.ConfigurationProperties;
 import org.eclipse.aether.RepositorySystemSession;
@@ -33,6 +34,7 @@
 import org.eclipse.aether.named.NamedLockKey;
 import org.eclipse.aether.named.providers.FileLockNamedLockFactory;
 import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.artifact.ArtifactIdUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -54,7 +56,7 @@ public final class NamedLockFactoryAdapter {
      */
     public static final String CONFIG_PROP_TIME = CONFIG_PROPS_PREFIX + "time";
 
-    public static final long DEFAULT_TIME = 30L;
+    public static final long DEFAULT_TIME = 900L;
 
     /**
      * The unit of maximum time amount to be blocked to obtain lock. Use TimeUnit enum names.
@@ -161,7 +163,7 @@ private AdaptedLockSyncContext(
             this.shared = shared;
             this.lockNaming = lockNaming;
             this.namedLockFactory = namedLockFactory;
-            this.time = getTime(session);
+            this.time = getTime(session, DEFAULT_TIME, CONFIG_PROP_TIME);
             this.timeUnit = getTimeUnit(session);
             this.retry = getRetry(session);
             this.retryWait = getRetryWait(session);
@@ -178,8 +180,8 @@ private AdaptedLockSyncContext(
             }
         }
 
-        private long getTime(final RepositorySystemSession session) {
-            return ConfigUtils.getLong(session, DEFAULT_TIME, CONFIG_PROP_TIME);
+        private long getTime(final RepositorySystemSession session, long defaultValue, String... keys) {
+            return ConfigUtils.getLong(session, defaultValue, keys);
         }
 
         private TimeUnit getTimeUnit(final RepositorySystemSession session) {
@@ -255,12 +257,48 @@ public void acquire(Collection<? extends Artifact> artifacts, Collection<? exten
             }
             // if we are here, means all attempts were unsuccessful: fail
             close();
-            IllegalStateException ex = new IllegalStateException("Could not acquire " + lockKind + " lock for "
-                    + namedLock.key().resources() + " using lock "
-                    + namedLock.key().name() + " in " + timeStr);
+            String message = "Could not acquire " + lockKind + " lock for "
+                    + lockSubjects(artifacts, metadatas) + " in " + timeStr
+                    + "; consider using '" + CONFIG_PROP_TIME
+                    + "' property to increase lock timeout to a value that fits your environment";
+            FailedToAcquireLockException ex = new FailedToAcquireLockException(shared, message);
             throw namedLockFactory.onFailure(ex);
         }
 
+        private String lockSubjects(
+                Collection<? extends Artifact> artifacts, Collection<? extends Metadata> metadatas) {
+            StringBuilder builder = new StringBuilder();
+            if (artifacts != null && !artifacts.isEmpty()) {
+                builder.append("artifacts: ")
+                        .append(artifacts.stream().map(ArtifactIdUtils::toId).collect(Collectors.joining(", ")));
+            }
+            if (metadatas != null && !metadatas.isEmpty()) {
+                if (builder.length() != 0) {
+                    builder.append("; ");
+                }
+                builder.append("metadata: ")
+                        .append(metadatas.stream().map(this::metadataSubjects).collect(Collectors.joining(", ")));
+            }
+            return builder.toString();
+        }
+
+        private String metadataSubjects(Metadata metadata) {
+            String name = "";
+            if (!metadata.getGroupId().isEmpty()) {
+                name += metadata.getGroupId();
+                if (!metadata.getArtifactId().isEmpty()) {
+                    name += ":" + metadata.getArtifactId();
+                    if (!metadata.getVersion().isEmpty()) {
+                        name += ":" + metadata.getVersion();
+                    }
+                }
+            }
+            if (!metadata.getType().isEmpty()) {
+                name += (name.isEmpty() ? "" : ":") + metadata.getType();
+            }
+            return name;
+        }
+
         @Override
         public void close() {
             while (!locks.isEmpty()) {

src/site/markdown/configuration.md

@@ -118,7 +118,7 @@ To modify this file, edit the template and regenerate.
 | `"aether.syncContext.named.redisson.configFile"` | `String` | Path to a Redisson configuration file in YAML format. Read official documentation for details. |  -  | 1.7.0 |  No  | Java System Properties |
 | `"aether.syncContext.named.retry"` | `Integer` | The amount of retries on time-out. |  `1`  | 1.7.0 |  No  | Session Configuration |
 | `"aether.syncContext.named.retry.wait"` | `Long` | The amount of milliseconds to wait between retries on time-out. |  `200l`  | 1.7.0 |  No  | Session Configuration |
-| `"aether.syncContext.named.time"` | `Long` | The maximum of time amount to be blocked to obtain lock. |  `30l`  | 1.7.0 |  No  | Session Configuration |
+| `"aether.syncContext.named.time"` | `Long` | The maximum of time amount to be blocked to obtain lock. |  `900l`  | 1.7.0 |  No  | Session Configuration |
 | `"aether.syncContext.named.time.unit"` | `String` | The unit of maximum time amount to be blocked to obtain lock. Use TimeUnit enum names. |  `"SECONDS"`  | 1.7.0 |  No  | Session Configuration |
 | `"aether.system.dependencyVisitor"` | `String` | A flag indicating which visitor should be used to "flatten" the dependency graph into list. In Maven 4 the default is new "levelOrder", while Maven 3 used "preOrder". This property accepts values "preOrder", "postOrder" and "levelOrder". |  `"levelOrder"`  | 2.0.0 |  No  | Session Configuration |
 | `"aether.transport.apache.followRedirects"` | `Boolean` | If enabled, Apache HttpClient will follow HTTP redirects. |  `true`  | 2.0.2 |  Yes  | Session Configuration |