rspec/rules/S2142/java/rule.adoc

103 lines
3.0 KiB
Plaintext
Raw Normal View History

== Why is this an issue?
2021-04-28 16:49:39 +02:00
``++InterruptedExceptions++`` should never be ignored in the code, and simply logging the exception counts in this case as "ignoring". The throwing of the ``++InterruptedException++`` clears the interrupted state of the Thread, so if the exception is not handled properly the information that the thread was interrupted will be lost. Instead, ``++InterruptedExceptions++`` should either be rethrown - immediately or after cleaning up the method's state - or the thread should be re-interrupted by calling ``++Thread.interrupt()++`` even if this is supposed to be a single-threaded application. Any other course of action risks delaying thread shutdown and loses the information that the thread was interrupted - probably without finishing its task.
Similarly, the ``++ThreadDeath++`` exception should also be propagated. According to its JavaDoc:
____
If ``++ThreadDeath++`` is caught by a method, it is important that it be rethrown so that the thread actually dies.
____
=== Noncompliant code example
2021-04-28 16:49:39 +02:00
2022-02-04 17:28:24 +01:00
[source,java]
2021-04-28 16:49:39 +02:00
----
public void run () {
try {
while (true) {
// do stuff
}
}catch (InterruptedException e) { // Noncompliant; logging is not enough
LOGGER.log(Level.WARN, "Interrupted!", e);
}
}
----
=== Compliant solution
2021-04-28 16:49:39 +02:00
2022-02-04 17:28:24 +01:00
[source,java]
2021-04-28 16:49:39 +02:00
----
public void run () {
try {
while (true) {
// do stuff
}
}catch (InterruptedException e) {
LOGGER.log(Level.WARN, "Interrupted!", e);
// Restore interrupted state...
Thread.currentThread().interrupt();
}
}
----
== Resources
2021-04-28 16:49:39 +02:00
* https://cwe.mitre.org/data/definitions/391[MITRE, CWE-391] - Unchecked Error Condition
ifdef::env-github,rspecator-view[]
'''
== Implementation Specification
(visible only on this page)
=== Message
Either re-interrupt this method or rethrow the "{InterruptedException/ThreadDeath}" that can be caught here.
=== Highlighting
* Primary: Catch parameter
* Secondary: Method call throwing "InterruptedException"
'''
== Comments And Links
(visible only on this page)
=== is related to: S5754
=== on 14 Oct 2014, 21:21:47 Freddy Mallet wrote:
@Ann, could you provide the source of this RSPEC because would like to double-check the main goal of this rule ? For sure here the code snippets are really misleading because we could have the feeling that when the execution of a Runnable class is interrupted, this exception can be caught in the ``++run++`` method which is not at all the case.
=== on 15 Oct 2014, 11:59:38 Ann Campbell wrote:
\[~freddy.mallet] \https://twitter.com/aparnachaudhary/status/520952677631807488
=== on 4 Sep 2019, 20:33:20 Réda Housni Alaoui wrote:
Hi,
I think the rule derived from this spec is too narrow.
Many people write ``++catch (Exception e)++`` in their applications.
Following this spec, IMO, that means that any ``++catch(Exception e)++`` must ALWAYS be preceded by a catch of InterruptedException like this
----
catch (InterruptedException e) {
Thread.currentThread().interrupt();
} catch (Exception e) {
//...
}{code}
 
----
endif::env-github,rspecator-view[]