Ivan Ponomarev created LANG-1817:
------------------------------------
Summary: UncheckedFutureImpl clears thread interrupt status when
wrapping InterruptedException
Key: LANG-1817
URL: https://issues.apache.org/jira/browse/LANG-1817
Project: Commons Lang
Issue Type: Bug
Reporter: Ivan Ponomarev
h3. Problem
{{UncheckedFutureImpl}} {{.get()}} and {{.get(long, TimeUnit)}} catch
{{InterruptedException}} and rethrow it as
{{{}UncheckedInterruptedException{}}}, but do not restore the thread’s
interrupt status.
Catching InterruptedException clears the {{interrupt}} flag. As a result,
callers observing {{Thread.currentThread().isInterrupted()}} after calling
{{UncheckedFuture.get()}} may incorrectly see {{{}false{}}}, even though an
interrupt was delivered.
For example,
{code:java}
class Worker implements Runnable {
private final UncheckedFuture<?> future;
Worker(UncheckedFuture<?> future) {
this.future = future;
}
@Override
public void run() {
while (!Thread.currentThread().isInterrupted()) {
// Block waiting for work or result
try {
future.get(); // <-- wrapped Future
doWork();
} catch (RuntimeException e) {
log.warn("Transient failure", e);
// continue loop
}
}
}
}
//Shutdown logic
executor.shutdownNow(); // interrupts worker threads
{code}
* Thread is interrupted.
* {{Future.get()}} throws {{{}InterruptedException{}}}.
* {{UncheckedFutureImpl.get()}} catches it and clears the {{{}interrupt{}}}.
It throws {{{}UncheckedInterruptedException{}}}.
* Caller catches or ignores the unchecked exception.
* Thread interrupt flag is now false.
* {{while (!Thread.currentThread().isInterrupted())}} continues running.
* {*}Even if the exception is not caught{*}, and the task returns a thread to
the pool as non-interrupted, then any higher-level cancellation logic that
relies on the flag won’t see it.
h3. Proposed fix
When wrapping {{{}InterruptedException{}}}, the interrupt status should be
preserved:
{code:java}
catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new UncheckedInterruptedException(e);
}
{code}
This matches standard Java concurrency best practices.
See Section 7.1 of "Java Concurrency in Practice" (Brian Goetz et al):
{quote}If [the task] is not simply going to propagate
{{InterruptedException}} to its caller, it should restore the interruption
status after
catching {{InterruptedException}} using {{Thread.currentThread().interrupt();}}
{quote}
See also
[code-review-checklists/java-concurrency/#IF.1|https://github.com/code-review-checklists/java-concurrency?tab=readme-ov-file#restore-interruption]
{quote}If some code propagates InterruptedException wrapped into another
exception (e. g. RuntimeException), is the interruption status of the current
thread restored before the wrapping exception is thrown?
{quote}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
