108 lines
3.0 KiB
Plaintext
108 lines
3.0 KiB
Plaintext
== Why is this an issue?
|
||
|
||
If an `InterruptedException` or a `ThreadDeath` error is not handled properly, the information that the thread was interrupted will be lost.
|
||
Handling this exception means either to re-throw it or manually re-interrupt the current thread by calling `Thread.interrupt()`.
|
||
Simply logging the exception is not sufficient and counts as ignoring it.
|
||
Between the moment the exception is caught and handled, is the right time to perform cleanup operations on the method's state, if needed.
|
||
|
||
=== What is the potential impact?
|
||
|
||
Failing to interrupt the thread (or to re-throw) risks delaying the thread shutdown and losing the information that the thread was interrupted - probably without finishing its task.
|
||
|
||
|
||
=== Noncompliant code example
|
||
|
||
[source,java]
|
||
----
|
||
public void run () {
|
||
try {
|
||
/*...*/
|
||
} catch (InterruptedException e) { // Noncompliant; logging is not enough
|
||
LOGGER.log(Level.WARN, "Interrupted!", e);
|
||
}
|
||
}
|
||
----
|
||
|
||
|
||
=== Compliant solution
|
||
|
||
[source,java]
|
||
----
|
||
public void run () {
|
||
try {
|
||
/* ... */
|
||
} catch (InterruptedException e) { // Compliant; the interrupted state is restored
|
||
LOGGER.log(Level.WARN, "Interrupted!", e);
|
||
/* Clean up whatever needs to be handled before interrupting */
|
||
Thread.currentThread().interrupt();
|
||
}
|
||
}
|
||
|
||
public void run () {
|
||
try {
|
||
/* ... */
|
||
} catch (ThreadDeath e) { // Compliant; the error is being re-thrown
|
||
LOGGER.log(Level.WARN, "Interrupted!", e);
|
||
/* Clean up whatever needs to be handled before re-throwing */
|
||
throw e;
|
||
}
|
||
}
|
||
----
|
||
|
||
|
||
== Resources
|
||
|
||
* CWE - https://cwe.mitre.org/data/definitions/391[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[]
|