This is an automated email from the ASF dual-hosted git repository.
cstamas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git
The following commit(s) were added to refs/heads/master by this push:
new a0d2f388 [MRESOLVER-370] Lock factory diagnostic on failure (#299)
a0d2f388 is described below
commit a0d2f388ab2307ae00e861b953d064e80fc27a02
Author: Tamas Cservenak <[email protected]>
AuthorDate: Tue Jun 13 10:39:18 2023 +0200
[MRESOLVER-370] Lock factory diagnostic on failure (#299)
The change "pulls" all `NamedLock` interface methods "level up" from
implementations to `NamedLockSupport`, to be able to augment instances at
`NamedLockSupport` level.
If diagnostic enabled:
* factory makes all named lock instances to gather "diag stats" (in memory!)
* on adapter failure, factory will dump out diagnostic state,
* each active lock (non closed) will dump it's state out (per thread with
"steps")
* output goes to lock factory INFO logger, potentially swarming console
By default, when "diagnostic" not turned on, no change really.
---
https://issues.apache.org/jira/browse/MRESOLVER-370
---
.../synccontext/named/NamedLockFactoryAdapter.java | 2 +-
.../org/eclipse/aether/named/NamedLockFactory.java | 13 +++++
.../named/providers/NoopNamedLockFactory.java | 6 +-
.../named/support/AdaptedSemaphoreNamedLock.java | 6 +-
.../aether/named/support/FileLockNamedLock.java | 6 +-
.../named/support/NamedLockFactorySupport.java | 52 +++++++++++++++++
.../aether/named/support/NamedLockSupport.java | 68 ++++++++++++++++++++++
.../named/support/ReadWriteLockNamedLock.java | 6 +-
.../src/site/markdown/index.md.vm | 12 ++++
.../aether/named/NamedLockFactoryTestSupport.java | 28 +++++++++
10 files changed, 186 insertions(+), 13 deletions(-)
diff --git
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java
index 246ba7bb..7a608c95 100644
---
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java
+++
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java
@@ -218,7 +218,7 @@ public final class NamedLockFactoryAdapter {
if (!illegalStateExceptions.isEmpty()) {
IllegalStateException ex = new IllegalStateException("Could
not acquire lock(s)");
illegalStateExceptions.forEach(ex::addSuppressed);
- throw ex;
+ throw namedLockFactory.onFailure(ex);
}
}
diff --git
a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/NamedLockFactory.java
b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/NamedLockFactory.java
index 56935637..e9ee3ea2 100644
---
a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/NamedLockFactory.java
+++
b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/NamedLockFactory.java
@@ -35,4 +35,17 @@ public interface NamedLockFactory {
* Performs a clean shut down of the factory.
*/
void shutdown();
+
+ /**
+ * Method to notify factory about locking failure, to make it possible to
provide more (factory specific)
+ * information about factory state when a locking operation failed.
Factory may alter provided failure or
+ * provide information via some other side effect (for example via
logging).
+ * <p>
+ * The default implementation merely does what happened before: adds no
extra information.
+ *
+ * @since TBD
+ */
+ default <E extends Throwable> E onFailure(E failure) {
+ return failure;
+ }
}
diff --git
a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/NoopNamedLockFactory.java
b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/NoopNamedLockFactory.java
index 67e0b14e..55460374 100644
---
a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/NoopNamedLockFactory.java
+++
b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/NoopNamedLockFactory.java
@@ -45,17 +45,17 @@ public class NoopNamedLockFactory extends
NamedLockFactorySupport {
}
@Override
- public boolean lockShared(final long time, final TimeUnit unit) {
+ protected boolean doLockShared(final long time, final TimeUnit unit) {
return true;
}
@Override
- public boolean lockExclusively(final long time, final TimeUnit unit) {
+ protected boolean doLockExclusively(final long time, final TimeUnit
unit) {
return true;
}
@Override
- public void unlock() {
+ protected void doUnlock() {
// no-op
}
}
diff --git
a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/AdaptedSemaphoreNamedLock.java
b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/AdaptedSemaphoreNamedLock.java
index 348d2968..f3be9665 100644
---
a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/AdaptedSemaphoreNamedLock.java
+++
b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/AdaptedSemaphoreNamedLock.java
@@ -66,7 +66,7 @@ public class AdaptedSemaphoreNamedLock extends
NamedLockSupport {
}
@Override
- public boolean lockShared(final long time, final TimeUnit unit) throws
InterruptedException {
+ protected boolean doLockShared(final long time, final TimeUnit unit)
throws InterruptedException {
Deque<Integer> perms = threadPerms.get();
if (!perms.isEmpty()) { // we already own shared or exclusive lock
perms.push(NONE);
@@ -80,7 +80,7 @@ public class AdaptedSemaphoreNamedLock extends
NamedLockSupport {
}
@Override
- public boolean lockExclusively(final long time, final TimeUnit unit)
throws InterruptedException {
+ protected boolean doLockExclusively(final long time, final TimeUnit unit)
throws InterruptedException {
Deque<Integer> perms = threadPerms.get();
if (!perms.isEmpty()) { // we already own shared or exclusive lock
if (perms.contains(EXCLUSIVE)) {
@@ -98,7 +98,7 @@ public class AdaptedSemaphoreNamedLock extends
NamedLockSupport {
}
@Override
- public void unlock() {
+ protected void doUnlock() {
Deque<Integer> steps = threadPerms.get();
if (steps.isEmpty()) {
throw new IllegalStateException("Wrong API usage: unlock without
lock");
diff --git
a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/FileLockNamedLock.java
b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/FileLockNamedLock.java
index 957e4666..e69dba93 100644
---
a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/FileLockNamedLock.java
+++
b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/FileLockNamedLock.java
@@ -83,12 +83,12 @@ public final class FileLockNamedLock extends
NamedLockSupport {
}
@Override
- public boolean lockShared(final long time, final TimeUnit unit) throws
InterruptedException {
+ protected boolean doLockShared(final long time, final TimeUnit unit)
throws InterruptedException {
return retry(time, unit, RETRY_SLEEP_MILLIS, this::doLockShared, null,
false);
}
@Override
- public boolean lockExclusively(final long time, final TimeUnit unit)
throws InterruptedException {
+ protected boolean doLockExclusively(final long time, final TimeUnit unit)
throws InterruptedException {
return retry(time, unit, RETRY_SLEEP_MILLIS, this::doLockExclusively,
null, false);
}
@@ -169,7 +169,7 @@ public final class FileLockNamedLock extends
NamedLockSupport {
}
@Override
- public void unlock() {
+ protected void doUnlock() {
criticalRegion.lock();
try {
Deque<Boolean> steps =
threadSteps.computeIfAbsent(Thread.currentThread(), k -> new ArrayDeque<>());
diff --git
a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockFactorySupport.java
b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockFactorySupport.java
index 17a8fbec..bb84afa3 100644
---
a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockFactorySupport.java
+++
b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockFactorySupport.java
@@ -18,6 +18,9 @@
*/
package org.eclipse.aether.named.support;
+import java.util.Deque;
+import java.util.HashMap;
+import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.AtomicInteger;
@@ -32,12 +35,35 @@ import static java.util.Objects.requireNonNull;
* Support class for {@link NamedLockFactory} implementations providing
reference counting.
*/
public abstract class NamedLockFactorySupport implements NamedLockFactory {
+ /**
+ * System property key to enable locking diagnostic collection.
+ *
+ * @since TBD
+ */
+ private static final boolean DIAGNOSTIC_ENABLED =
Boolean.getBoolean("aether.named.diagnostic.enabled");
+
protected final Logger logger = LoggerFactory.getLogger(getClass());
private final ConcurrentMap<String, NamedLockHolder> locks;
+ private final boolean diagnosticEnabled;
+
public NamedLockFactorySupport() {
+ this(DIAGNOSTIC_ENABLED);
+ }
+
+ public NamedLockFactorySupport(boolean diagnosticEnabled) {
this.locks = new ConcurrentHashMap<>();
+ this.diagnosticEnabled = diagnosticEnabled;
+ }
+
+ /**
+ * Returns {@code true} if factory diagnostic collection is enabled.
+ *
+ * @since TBD
+ */
+ public boolean isDiagnosticEnabled() {
+ return diagnosticEnabled;
}
@Override
@@ -57,6 +83,32 @@ public abstract class NamedLockFactorySupport implements
NamedLockFactory {
// override if needed
}
+ @Override
+ public <E extends Throwable> E onFailure(E failure) {
+ if (isDiagnosticEnabled()) {
+ Map<String, NamedLockHolder> locks = new HashMap<>(this.locks); //
copy
+ int activeLocks = locks.size();
+ logger.info("Diagnostic dump of lock factory");
+ logger.info("===============================");
+ logger.info("Implementation: {}", getClass().getName());
+ logger.info("Active locks: {}", activeLocks);
+ logger.info("");
+ if (activeLocks > 0) {
+ for (Map.Entry<String, NamedLockHolder> entry :
locks.entrySet()) {
+ String name = entry.getKey();
+ int refCount = entry.getValue().referenceCount.get();
+ NamedLockSupport lock = entry.getValue().namedLock;
+ logger.info("Name: {}", name);
+ logger.info("RefCount: {}", refCount);
+ Map<Thread, Deque<String>> diag = lock.diagnosticState();
+ diag.forEach((key, value) -> logger.info(" {} -> {}",
key, value));
+ }
+ logger.info("");
+ }
+ }
+ return failure;
+ }
+
public void closeLock(final String name) {
locks.compute(name, (k, v) -> {
if (v != null && v.decRef() == 0) {
diff --git
a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java
b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java
index 9bd966df..67c2208b 100644
---
a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java
+++
b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java
@@ -18,6 +18,13 @@
*/
package org.eclipse.aether.named.support;
+import java.util.ArrayDeque;
+import java.util.Collections;
+import java.util.Deque;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
+
import org.eclipse.aether.named.NamedLock;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -32,9 +39,12 @@ public abstract class NamedLockSupport implements NamedLock {
private final NamedLockFactorySupport factory;
+ private final ConcurrentHashMap<Thread, Deque<String>> diagnosticState; //
non-null only if diag enabled
+
public NamedLockSupport(final String name, final NamedLockFactorySupport
factory) {
this.name = name;
this.factory = factory;
+ this.diagnosticState = factory.isDiagnosticEnabled() ? new
ConcurrentHashMap<>() : null;
}
@Override
@@ -42,8 +52,66 @@ public abstract class NamedLockSupport implements NamedLock {
return name;
}
+ @Override
+ public boolean lockShared(long time, TimeUnit unit) throws
InterruptedException {
+ if (diagnosticState != null) {
+ diagnosticState
+ .computeIfAbsent(Thread.currentThread(), k -> new
ArrayDeque<>())
+ .push("shared");
+ }
+ return doLockShared(time, unit);
+ }
+
+ protected abstract boolean doLockShared(long time, TimeUnit unit) throws
InterruptedException;
+
+ @Override
+ public boolean lockExclusively(long time, TimeUnit unit) throws
InterruptedException {
+ if (diagnosticState != null) {
+ diagnosticState
+ .computeIfAbsent(Thread.currentThread(), k -> new
ArrayDeque<>())
+ .push("exclusive");
+ }
+ return doLockExclusively(time, unit);
+ }
+
+ protected abstract boolean doLockExclusively(long time, TimeUnit unit)
throws InterruptedException;
+
+ @Override
+ public void unlock() {
+ doUnlock();
+ if (diagnosticState != null) {
+ diagnosticState
+ .computeIfAbsent(Thread.currentThread(), k -> new
ArrayDeque<>())
+ .pop();
+ }
+ }
+
+ protected abstract void doUnlock();
+
@Override
public void close() {
+ doClose();
+ }
+
+ protected void doClose() {
factory.closeLock(name);
}
+
+ /**
+ * Returns the diagnostic state (if collected) or empty map, never {@code
null}.
+ *
+ * @since TBD
+ */
+ public Map<Thread, Deque<String>> diagnosticState() {
+ if (diagnosticState != null) {
+ return diagnosticState;
+ } else {
+ return Collections.emptyMap();
+ }
+ }
+
+ @Override
+ public String toString() {
+ return getClass().getSimpleName() + "{" + "name='" + name + '\'' + '}';
+ }
}
diff --git
a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/ReadWriteLockNamedLock.java
b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/ReadWriteLockNamedLock.java
index 54b1f61e..df49e958 100644
---
a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/ReadWriteLockNamedLock.java
+++
b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/ReadWriteLockNamedLock.java
@@ -53,7 +53,7 @@ public class ReadWriteLockNamedLock extends NamedLockSupport {
}
@Override
- public boolean lockShared(final long time, final TimeUnit unit) throws
InterruptedException {
+ protected boolean doLockShared(final long time, final TimeUnit unit)
throws InterruptedException {
Deque<Step> steps = threadSteps.get();
if (readWriteLock.readLock().tryLock(time, unit)) {
steps.push(Step.SHARED);
@@ -63,7 +63,7 @@ public class ReadWriteLockNamedLock extends NamedLockSupport {
}
@Override
- public boolean lockExclusively(final long time, final TimeUnit unit)
throws InterruptedException {
+ protected boolean doLockExclusively(final long time, final TimeUnit unit)
throws InterruptedException {
Deque<Step> steps = threadSteps.get();
if (!steps.isEmpty()) { // we already own shared or exclusive lock
if (!steps.contains(Step.EXCLUSIVE)) {
@@ -78,7 +78,7 @@ public class ReadWriteLockNamedLock extends NamedLockSupport {
}
@Override
- public void unlock() {
+ protected void doUnlock() {
Deque<Step> steps = threadSteps.get();
if (steps.isEmpty()) {
throw new IllegalStateException("Wrong API usage: unlock without
lock");
diff --git a/maven-resolver-named-locks/src/site/markdown/index.md.vm
b/maven-resolver-named-locks/src/site/markdown/index.md.vm
index 24ed4760..17486e77 100644
--- a/maven-resolver-named-locks/src/site/markdown/index.md.vm
+++ b/maven-resolver-named-locks/src/site/markdown/index.md.vm
@@ -69,3 +69,15 @@ Out of the box, name mapper implementations are the
following:
- `file-gav` implemented in
`org.eclipse.aether.internal.impl.synccontext.named.FileGAVNameMapper`.
Note: the `file-gav` name mapper MUST be used with `file-lock` named locking,
no other mapper will work with it.
+
+${esc.hash}${esc.hash} Diagnostic collection in case of failures
+
+If you experience locking failures, to get access to full dump (may be huge in
case of big builds) on failure,
+enable lock diagnostic collection. When enabled, "diagnostic dump" will be
emitted by factory
+to INFO log (console by default). Please note, that enabling diagnostic dump
may increase heap requirement (as
+diagnostic is collected in memory).
+
+To enable diagnostic collection, set Java System Property
`aether.named.diagnostic.enabled` to `true`.
+
+If diagnostic collection is not enabled, Resolver will "just" fail, telling
about failed lock, but not reveal any
+information about other active locks and threads.
\ No newline at end of file
diff --git
a/maven-resolver-named-locks/src/test/java/org/eclipse/aether/named/NamedLockFactoryTestSupport.java
b/maven-resolver-named-locks/src/test/java/org/eclipse/aether/named/NamedLockFactoryTestSupport.java
index 471d308a..72e07e4e 100644
---
a/maven-resolver-named-locks/src/test/java/org/eclipse/aether/named/NamedLockFactoryTestSupport.java
+++
b/maven-resolver-named-locks/src/test/java/org/eclipse/aether/named/NamedLockFactoryTestSupport.java
@@ -43,6 +43,34 @@ public abstract class NamedLockFactoryTestSupport {
return testName.getMethodName();
}
+ @Test(expected = IllegalStateException.class)
+ public void testFailure() throws InterruptedException {
+ // note: set system property "aether.named.diagnostic.enabled" to
"true" to have log output
+ // this test does NOT assert its presence, only the proper flow
+ Thread t1 = new Thread(() -> {
+ try {
+ namedLockFactory.getLock(lockName()).lockShared(1L,
TimeUnit.MINUTES);
+ namedLockFactory.getLock(lockName()).lockShared(1L,
TimeUnit.MINUTES);
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
+ }
+ });
+ Thread t2 = new Thread(() -> {
+ try {
+ namedLockFactory.getLock(lockName()).lockShared(1L,
TimeUnit.MINUTES);
+ namedLockFactory.getLock(lockName()).lockShared(1L,
TimeUnit.MINUTES);
+ namedLockFactory.getLock(lockName()).lockShared(1L,
TimeUnit.MINUTES);
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
+ }
+ });
+ t1.start();
+ t2.start();
+ t1.join();
+ t2.join();
+ throw namedLockFactory.onFailure(new IllegalStateException("failure"));
+ }
+
@Test
public void refCounting() {
final String name = lockName();
