False positive for Eclipse SonarLint java:S2142 '"InterruptedException" should not be ignored'

I couldn’t find an entry for the following where Sonarlint for Eclipse version 2019-03 (4.11.0)/build id: 20190314-1200 reports java:S2142 (“InterruptedException” should not be ignored) for the code:

  public void start() {
    boolean distribute = true;
    do {
      try {
      } catch (InterruptedException ie) { //java:S2142 raised on this line, highlighting "InterruptedException".
        distribute = false;
    } while (distribute);

It is reported as a “Major Bug”. However:

  • There is no bug in this case - the exception is handled and not ignored. To further emphasise this, I can add that this is code called from the main() method in the main thread (which in the non-minimal example does something in the loop, of course) shortly before program exit.

  • Following the recommendation in the description, calling Thread.interrupt() from the catch block may cause errors, in other cases either introducing a threading bug where previously there was none, or produce unit testing failures, which appear unrelated to the cause, both not necessarily trivial to track down, especially because a linter recommendation is unlikely to be the first place to look (compliment!).

For example, in my case it would not compromise the program, but in stead interfere with JUnit tests, essentially giving problems next time some code checks the interruption state, in this case failures in test cases in another test class when trying to start WireMock’s jetty server.

I understand that the following may be better in most ways in this and many cases:

  public void start() {
    do {
      try {
      } catch (InterruptedException ie) {
    } while (!Thread.interrupted());

but the first snippet does not contain a bug, thus if SonarLint does not analyse the whole program context, which is perfectly reasonable, it may be better to flag this as a warning or code smell IMHO.

Hello Nikolas.
Welcome to Sonarsource community!
Thank you for your detailed report. We are working on this. We will post decision on this as soon as we have it.

Hi Nikolas,

When an InterruptedException is raised, the thread should just clean up and exit instead of continuing.
By not rethrowing the InterruptedException or interrupting the current thread, there’s a risk to ignore the exception and continue as if nothing had happened.
In a method main, if you catch InterruptedException and just clean up and exit, I agree it 's ok. But it’s hard from the java analyzer point of view to distinguish if a code is just some cleanup or if it’s the business as usual. Let’s say, it’s a limitation of the rule. The rule can detect calls to throw or interrupt() easily, but it’s hard to understand how fast the program will end.
In a program, I expect only one or two locations to be impacted by this S2142 false-positive, so the effort to fix them would be huge and the benefit small.
It’s a configuration where marking them as won't fix in SonarQube should not be too annoying. (Or using @SuppressWarnings("java:S2142") on the method)

Good evening/morning

Thank you for the reply - I have only just seen this now.

I think SonarLint does a good job in general, but I am of the strong opinion that calling something that is not - repeat - not a bug a “Major Bug” and suggesting a fix that can introduce problems is too aggressive.

It is not the problem of linter users’ as such that cases are hard to call correctly using code analysis and there is more than one way of doing things.

I think it is important we can trust the linter. You could perhaps note in the rule explanation that “it is not always a bug”.
If that seems contradictory… then it is perhaps better to mark it as a warning or code smell with the suggestion for how to fix.

Kind regards,
mvh. Nikolas Andersen

I created this ticket SONARJAVA-3886 to improve the message and the description of the rule, so the rule does not strongly state that there’s a bug.

1 Like

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.