InterruptedException should not be caught and handled with other exceptions

Sometimes methods throw mulitple exceptions including InterruptedException. Consider for example

public void org.eclipse.jface.dialogs.ProgressMonitorDialog.run(boolean, boolean, IRunnableWithProgress) throws InvocationTargetException,
			InterruptedException;

from Jface. This could result in handling both exceptions using a multi-catch:

try {
  diaglog.run(...);
} catch (InvocationTargetException | InterruptedException e) {
  ...
}

Following guidance of rule S2142 handling the InterruptedException could lead to the following code:

try {
  dialog.run(...);
} catch (InvocationTargetException | InterruptedException e) {
  Thread.currentThread().interrupt();
}

However, now the current thread is interrupted even if an exception unrelated to InterruptedException is thrown. This leads to bugs which are difficult to track because code executing at a later time which checks the interrupt flag might stop execution.
Valid handling could look like

try {
  dialog.run(...);
} catch (InvocationTargetException e) {
  ...
} catch (InterruptedException e) {
  Thread.currentThread().interrupt();
}

This rule should be classified as bug because handling any exception the same as InterruptedException is always wrong.

There are no exceptions to this rule: No exception should be handled the same as InterruptedException and error handling for InterruptedException is never the same as for any other exception.

Hello @oliver
thanks for raising this topic.
Let me try to rephrase a bit to make sure I have it right.

  1. You are promoting here the addition of a new rule in order to prevent the handling of any exception with InterruptedException
  2. You interpret S2142 as possibly misleading because you believe it may lead to a handling of an InterruptedException as part of a multi-catch statement.

Is that correct?
Although I see some merit with point 1 and your rule suggestion, I have some doubts about the 2nd one. S2142 does not mentions this corner case and I don’t believe it should prevent any developer from applying some common sense when writing his Exception handling.

Would you want to attract more interest from the Java developers community, I believe you should rephrase your post as a clearer rule suggestion.
Linking the good practice enforcement that you are proposing to some wrong guidance given by the S2142 rule documentation is a debatable point that is preventing your idea to be understood and weighted by the Java developers I believe.
WDYT?

Best.
Sylvain

Hi Sylvain,

thanks for your time and sorry for the late reply.

  1. Yes, that is my intention
  2. Yes, I believe that is possible. I’ve already seen that in action.

Our code base predates our use of SonarQube, so this is a common pattern:

try {
  dialog.run(...);
} catch (InvocationTargetException | InterruptedException e) {
  ...
}

After introducing SonarQube issues popped up, claiming that InterruptedException needs additional handling. Namely, be re-interrupting. When someone came by this code he/she saw there is an issue to fix and slapped the re-interrupt onto the existing handling. Thus it became:

try {
  dialog.run(...);
} catch (InvocationTargetException | InterruptedException e) {
  ...
  Thread.currentThread().interrupt();
}

I agree with you that common sense could have prevented that, but so would have a rule explicitely prohibiting such an implementation. Besides, by arguing to rely on common sense I could simply deactivate most of the rules.

An alternative to the currently proposed rule would be to prohibit re-interrupts in multi-catch blocks. Instead of checking if InterruptedException is part of a multi-catch the rule would only trigger if interrupt() is called inside the block.

Hello @oliver
thanks for the additional explanations. This makes sense to me now and was duly noted as a rule suggestion.
In addition to your proposed rule addition, maybe you could also consider activating rule S1166, which would make sure the exception is logged or passed forward before the thread interruption. This rule is known to be quite noisy though, which is why it is not activated by default…

1 Like

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