[
https://issues.apache.org/jira/browse/LANG-1817?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gary D. Gregory resolved LANG-1817.
-----------------------------------
Fix Version/s: 3.21.0
Resolution: Fixed
> 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
> Priority: Major
> Fix For: 3.21.0
>
>
> 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}
> Proposed fix: https://github.com/apache/commons-lang/pull/1590
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
