Skip to content

Commit fa39240

Browse files
committed
[LANG-1817] UncheckedFutureImpl clears thread interrupt status when
wrapping InterruptedException. Partial application of patch #1590.
1 parent 3a055ba commit fa39240

3 files changed

Lines changed: 67 additions & 0 deletions

File tree

src/changes/changes.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ The <action> type attribute can be add,update,fix,remove.
114114
<action type="fix" dev="ggregory" due-to="Ivan Ponomarev, Gary Gregory">Fix StringIndexOutOfBoundsException message in StrBuilder.append(char[], int, int).</action>
115115
<action issue="LANG-1818" type="fix" dev="ggregory" due-to="Ivan Ponomarev, Gary Gregory">Fix ClassUtils.getShortClassName(Class) to correctly handle $ in valid class names #1591.</action>
116116
<action type="fix" dev="ggregory" due-to="Ivan Ponomarev, Gary Gregory">ThreadUtils.sleepQuietly(Duration) now restores the current thread's interrupt flag when catching InterruptedException.</action>
117+
<action issue="LANG-1817" type="fix" dev="ggregory" due-to="Ivan Ponomarev, Gary Gregory">UncheckedFutureImpl clears thread interrupt status when wrapping InterruptedException #1590.</action>
117118
<!-- ADD -->
118119
<action type="add" dev="ggregory" due-to="Gary Gregory">Add JavaVersion.JAVA_27.</action>
119120
<action type="add" dev="ggregory" due-to="Gary Gregory">Add SystemUtils.IS_JAVA_27.</action>

src/main/java/org/apache/commons/lang3/concurrent/UncheckedFutureImpl.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ public V get() {
4242
try {
4343
return super.get();
4444
} catch (final InterruptedException e) {
45+
Thread.currentThread().interrupt();
4546
throw new UncheckedInterruptedException(e);
4647
} catch (final ExecutionException e) {
4748
throw new UncheckedExecutionException(e);
@@ -53,6 +54,7 @@ public V get(final long timeout, final TimeUnit unit) {
5354
try {
5455
return super.get(timeout, unit);
5556
} catch (final InterruptedException e) {
57+
Thread.currentThread().interrupt();
5658
throw new UncheckedInterruptedException(e);
5759
} catch (final ExecutionException e) {
5860
throw new UncheckedExecutionException(e);

src/test/java/org/apache/commons/lang3/concurrent/UncheckedFutureTest.java

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,20 @@
1818
package org.apache.commons.lang3.concurrent;
1919

2020
import static org.junit.jupiter.api.Assertions.assertEquals;
21+
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
2122
import static org.junit.jupiter.api.Assertions.assertThrows;
23+
import static org.junit.jupiter.api.Assertions.assertTrue;
2224

2325
import java.util.Arrays;
2426
import java.util.List;
27+
import java.util.concurrent.CountDownLatch;
2528
import java.util.concurrent.ExecutionException;
2629
import java.util.concurrent.Future;
2730
import java.util.concurrent.TimeUnit;
2831
import java.util.concurrent.TimeoutException;
32+
import java.util.concurrent.atomic.AtomicBoolean;
33+
import java.util.concurrent.atomic.AtomicReference;
34+
import java.util.function.Consumer;
2935
import java.util.stream.Collectors;
3036

3137
import org.apache.commons.lang3.AbstractLangTest;
@@ -121,4 +127,62 @@ void testOnFuture() {
121127
assertEquals("Z", UncheckedFuture.on(new TestFuture<>("Z")).get());
122128
}
123129

130+
131+
@Test
132+
void interruptFlagIsPreservedOnGet() throws Exception {
133+
assertInterruptPreserved(UncheckedFuture::get);
134+
}
135+
136+
@Test
137+
void interruptFlagIsPreservedOnGetWithTimeout() throws Exception {
138+
assertInterruptPreserved(uf -> uf.get(1, TimeUnit.DAYS));
139+
}
140+
141+
private static void assertInterruptPreserved(Consumer<UncheckedFuture<Integer>> call) throws Exception {
142+
final CountDownLatch enteredGet = new CountDownLatch(1);
143+
final Future<Integer> blockingFuture = new AbstractFutureProxy<Integer>(ConcurrentUtils.constantFuture(42)) {
144+
private final CountDownLatch neverRelease = new CountDownLatch(1);
145+
146+
@Override
147+
public Integer get() throws InterruptedException {
148+
enteredGet.countDown();
149+
neverRelease.await();
150+
throw new AssertionError("We should not get here");
151+
}
152+
153+
@Override
154+
public Integer get(long timeout, TimeUnit unit) throws InterruptedException {
155+
enteredGet.countDown();
156+
neverRelease.await();
157+
throw new AssertionError("We should not get here");
158+
}
159+
160+
@Override
161+
public boolean isDone() {
162+
return false;
163+
}
164+
165+
};
166+
final UncheckedFuture<Integer> uf = UncheckedFuture.on(blockingFuture);
167+
final AtomicReference<Throwable> thrown = new AtomicReference<>();
168+
final AtomicBoolean interruptObserved = new AtomicBoolean(false);
169+
final Thread worker = new Thread(() -> {
170+
try {
171+
call.accept(uf);
172+
thrown.set(new AssertionError("We should not get here"));
173+
} catch (Throwable e) {
174+
interruptObserved.set(Thread.currentThread().isInterrupted());
175+
thrown.set(e);
176+
}
177+
}, "unchecked-future-test-worker");
178+
worker.start();
179+
assertTrue(enteredGet.await(2, TimeUnit.SECONDS), "Worker did not enter Future.get() in time");
180+
worker.interrupt();
181+
worker.join();
182+
final Throwable t = thrown.get();
183+
assertInstanceOf(UncheckedInterruptedException.class, t, "Unexpected exception: " + t);
184+
assertInstanceOf(InterruptedException.class, t.getCause(), "Cause should be InterruptedException");
185+
assertTrue(interruptObserved.get(), "Interrupt flag was not restored by the wrapper");
186+
}
187+
124188
}

0 commit comments

Comments
 (0)