[java] Add DeadlockTest for verifying #5293

- Improve logging for Parselock

Refs #5293
This commit is contained in:
Andreas Dangel
2024-10-31 17:04:19 +01:00
parent e15c05721e
commit a1996554d8
6 changed files with 114 additions and 12 deletions

View File

@ -84,12 +84,7 @@ final class ClassStub implements JClassSymbol, AsmStub, AnnotationOwner {
this.resolver = resolver;
this.names = new Names(internalName);
this.parseLock = new ParseLock() {
// note to devs: to debug the parsing logic you might have
// to replace the implementation of toString temporarily,
// otherwise an IDE could call toString just to show the item
// in the debugger view (which could cause parsing of the class file).
this.parseLock = new ParseLock("ClassStub:" + internalName) {
@Override
protected boolean doParse() throws IOException {
try (InputStream instream = loader.getInputStream()) {

View File

@ -46,9 +46,9 @@ abstract class GenericSigBase<T extends JTypeParameterOwnerSymbol & AsmStub> {
protected List<JTypeVar> typeParameters;
private final ParseLock lock;
protected GenericSigBase(T ctx) {
protected GenericSigBase(T ctx, String parseLockName) {
this.ctx = ctx;
this.lock = new ParseLock() {
this.lock = new ParseLock(parseLockName) {
@Override
protected boolean doParse() {
GenericSigBase.this.doParse();
@ -116,7 +116,7 @@ abstract class GenericSigBase<T extends JTypeParameterOwnerSymbol & AsmStub> {
@Nullable String signature, // null if doesn't use generics in header
@Nullable String superInternalName, // null if this is the Object class
String[] interfaces) {
super(ctx);
super(ctx, "LazyClassSignature:" + ctx.getInternalName() + "[" + signature + "]");
this.signature = signature;
this.rawItfs = CollectionUtil.map(interfaces, ctx.getResolver()::resolveFromInternalNameCannotFail);
@ -233,7 +233,7 @@ abstract class GenericSigBase<T extends JTypeParameterOwnerSymbol & AsmStub> {
@Nullable String genericSig,
@Nullable String[] exceptions,
boolean skipFirstParam) {
super(ctx);
super(ctx, "LazyMethodType:" + (genericSig != null ? genericSig : descriptor));
this.signature = genericSig != null ? genericSig : descriptor;
// generic signatures already omit the synthetic param
this.skipFirstParam = skipFirstParam && genericSig == null;

View File

@ -35,7 +35,7 @@ class ModuleStub implements JModuleSymbol, AsmStub, AnnotationOwner {
this.resolver = resolver;
this.moduleName = moduleName;
this.parseLock = new ParseLock() {
this.parseLock = new ParseLock("ModuleStub:" + moduleName) {
@Override
protected boolean doParse() throws IOException {
try (InputStream instream = loader.getInputStream()) {

View File

@ -17,15 +17,30 @@ abstract class ParseLock {
private static final Logger LOG = LoggerFactory.getLogger(ParseLock.class);
private volatile ParseStatus status = ParseStatus.NOT_PARSED;
private final String name;
protected ParseLock(String name) {
this.name = name;
}
public void ensureParsed() {
getFinalStatus();
}
private void logParseLockTrace(String prefix) {
if (LOG.isTraceEnabled()) {
LOG.trace("{} {}: {}", Thread.currentThread().getName(), String.format("%-15s", prefix), this);
}
}
private ParseStatus getFinalStatus() {
ParseStatus status = this.status;
if (!status.isFinished) {
logParseLockTrace("waiting on");
synchronized (this) {
logParseLockTrace("locked");
status = this.status;
if (status == ParseStatus.NOT_PARSED) {
this.status = ParseStatus.BEING_PARSED;
@ -54,6 +69,7 @@ abstract class ParseLock {
throw new IllegalStateException("Thread is reentering the parse lock");
}
}
logParseLockTrace("released");
}
return status;
}
@ -85,7 +101,7 @@ abstract class ParseLock {
@Override
public String toString() {
return "ParseLock{status=" + status + '}';
return "ParseLock{name=" + name + ",status=" + status + '}';
}
private enum ParseStatus {

View File

@ -0,0 +1,86 @@
/*
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.symbols;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
import net.sourceforge.pmd.lang.java.JavaParsingHelper;
import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
/**
* Tests to help analyze [java] Deadlock when executing PMD in multiple threads #5293.
*
* @see <a href="https://github.com/pmd/pmd/issues/5293">[java] Deadlock when executing PMD in multiple threads #5293</a>
*/
class DeadlockTest {
abstract static class Outer<T> implements GenericInterface<Outer<T>, GenericClass<T>> {
// must be a nested class, that is reusing the type param T of the outer class
abstract static class Inner<T> {
Inner(Outer<T> grid) { }
}
}
static class GenericBaseClass<T> { }
interface GenericInterface<T, S> { }
abstract static class GenericClass<T> extends GenericBaseClass<Outer.Inner<T>> { }
@Test
@Timeout(2)
void parseWithoutDeadlock() throws InterruptedException {
/*
Deadlock:
t1 -> locks parse for Outer.Inner and waits for parse lock for Outer
t2 -> locks parse for Outer, locks parse for GenericInterface and then waits for parse lock for Outer.Inner
In order to reproduce the deadlock reliably, add the following piece into ParseLock, just at the beginning
of the synchronized block (line 42):
// t1 needs to wait after having the lock, so that t2 can go on and wait on the same lock
if (Thread.currentThread().getName().equals("t1") && this.toString().contains("LazyClassSignature:net/sourceforge/pmd/lang/java/symbols/DeadlockTest$Outer$Inner[<T:L")) {
try {
Thread.sleep(500);
} catch (InterruptedException ignored) {
}
}
*/
Thread t1 = new Thread(() -> {
ASTCompilationUnit class1 = JavaParsingHelper.DEFAULT.parse(
"package net.sourceforge.pmd.lang.java.symbols;\n"
+ "import net.sourceforge.pmd.lang.java.symbols.DeadlockTest.Outer;\n"
+ " class Class1 {\n"
+ " public static <T> Outer.Inner<T> newInner(Outer<T> grid) {\n"
+ " return null;\n"
+ " }\n"
+ " }\n"
);
assertNotNull(class1);
}, "t1");
Thread t2 = new Thread(() -> {
ASTCompilationUnit class2 = JavaParsingHelper.DEFAULT.parse(
"package net.sourceforge.pmd.lang.java.symbols;\n"
+ "import net.sourceforge.pmd.lang.java.symbols.DeadlockTest.Outer;\n"
+ " class Class2<M> {\n"
+ " protected Outer<M> theOuter;\n"
+ " }\n"
);
assertNotNull(class2);
}, "t2");
t1.start();
t2.start();
t1.join();
t2.join();
}
}

View File

@ -0,0 +1,5 @@
#
# BSD-style license; for more info see http://pmd.sourceforge.net/license.html
#
#org.slf4j.simpleLogger.log.net.sourceforge.pmd.lang.java.symbols.internal.asm.ParseLock=trace