16f6c0aecf
Inline adoc files when they are included exactly once. Also fix language tags because this inlining gives us better information on what language the code is written in.
103 lines
3.0 KiB
Plaintext
103 lines
3.0 KiB
Plaintext
== Why is this an issue?
|
||
|
||
``++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
|
||
|
||
[source,java]
|
||
----
|
||
public void run () {
|
||
try {
|
||
while (true) {
|
||
// do stuff
|
||
}
|
||
}catch (InterruptedException e) { // Noncompliant; logging is not enough
|
||
LOGGER.log(Level.WARN, "Interrupted!", e);
|
||
}
|
||
}
|
||
----
|
||
|
||
|
||
=== Compliant solution
|
||
|
||
[source,java]
|
||
----
|
||
public void run () {
|
||
try {
|
||
while (true) {
|
||
// do stuff
|
||
}
|
||
}catch (InterruptedException e) {
|
||
LOGGER.log(Level.WARN, "Interrupted!", e);
|
||
// Restore interrupted state...
|
||
Thread.currentThread().interrupt();
|
||
}
|
||
}
|
||
----
|
||
|
||
|
||
== Resources
|
||
|
||
* 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[]
|