Deadlock on ProgressReport

[dbolkensteyn]

I observed a deadlock on ProgressReport during the execution of tests on a Java custom rules plugin written against SonarQube LTS 8.9 and sonar-java version 6.15.1.26025 which depends on sonar-analyzer-commons version 1.15.0.699.

Here is the jstack dump:

"Report about progress of Java AST analyzer" #13 daemon prio=5 os_prio=0 tid=0x000000001946b000 nid=0x6554 waiting on condition [0x000000001a61f000]
   java.lang.Thread.State: TIMED_WAITING (sleeping)
        at java.lang.Thread.sleep(Native Method)
        at org.sonarsource.analyzer.commons.ProgressReport.run(ProgressReport.java:61)
        at java.lang.Thread.run(Thread.java:748)

"main" #1 prio=5 os_prio=0 tid=0x0000000001bde800 nid=0x818 in Object.wait() [0x0000000001cdd000]
   java.lang.Thread.State: WAITING (on object monitor)
        at java.lang.Object.wait(Native Method)
        - waiting on <0x00000000d5a78c38> (a java.lang.Thread)
        at java.lang.Thread.join(Thread.java:1252)
        - locked <0x00000000d5a78c38> (a java.lang.Thread)
        at java.lang.Thread.join(Thread.java:1326)
        at org.sonarsource.analyzer.commons.ProgressReport.join(ProgressReport.java:116)
        at org.sonarsource.analyzer.commons.ProgressReport.stop(ProgressReport.java:106)
        - locked <0x00000000d5a78be8> (a org.sonarsource.analyzer.commons.ProgressReport)
        at org.sonar.java.ast.JavaAstScanner.scan(JavaAstScanner.java:86)
        at org.sonar.java.checks.verifier.InternalCheckVerifier.verifyAll(InternalCheckVerifier.java:218)
        at org.sonar.java.checks.verifier.InternalCheckVerifier.verifyNoIssues(InternalCheckVerifier.java:196)

Now this seems to be quite a tricky state to be in:

1. ProgressReport.stop() has been called, which interrupted ProgressReport's thread
2. ProgressReport's thread is however still looping in and calling sleep()

The Javadoc for [Thread.interrupted()](https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html#interrupted--) says:

A thread interruption ignored because a thread was not alive at the time of the interrupt will be reflected by this method returning false.

As this behavior occurs only in a unit test scanning a single file (which is fast), one explanation might be that stop() is being called before ProgressReport's thread becomes alive, thus turning the interruption sent by stop() into a no-operation. This deadlock is observable from one machine (not all) and there was at least one successful run which did not encounter this deadlock.

It might be safer to rely on a custom flag rather than the thread interruption status to exit from the progress report thread, given that the later depends on native methods of the JVM which seem hard to understand and which might slightly change between releases.

[alban-auzeill]

Hi Dinesh, thanks for the feedback! I'm trying to fix this issue in PR #152, can you confirm this PR fixes your case?

[dbolkensteyn]

Thank you @alban-auzeill for this quick fix. I will not be able to test this fix before quite some time, however looking at your PR I am confident it will resolve this issue. I suggest to merge the PR and close this issue: I will open a new one in the unlikely case the problem is still there.

[dbolkensteyn]

Great job @alban-auzeill ; I have just managed to verify that this issue is now indeed fixed. Well done and thank you!