Skip to content

Commit 3db15b6

Browse files
committed
Timeout stops threads if they are uninterruptible.
If a thread does not respond to interrupt() we'll try to stop() it after twice the specified timeout. This uses the deprecated Thread.stop() method, but for a testing tool like Cucumber that should be ok. Ref #343.
1 parent 6bffa75 commit 3db15b6

File tree

2 files changed

+65
-35
lines changed

2 files changed

+65
-35
lines changed

core/src/main/java/cucumber/runtime/Timeout.java

+17-3
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
package cucumber.runtime;
22

33
import java.util.concurrent.Executors;
4-
import java.util.concurrent.ScheduledFuture;
54
import java.util.concurrent.ScheduledExecutorService;
5+
import java.util.concurrent.ScheduledFuture;
66
import java.util.concurrent.TimeUnit;
77
import java.util.concurrent.TimeoutException;
88
import java.util.concurrent.atomic.AtomicBoolean;
@@ -16,21 +16,35 @@ public static <T> T timeout(Callback<T> callback, long timeoutMillis) throws Thr
1616
final AtomicBoolean done = new AtomicBoolean();
1717

1818
ScheduledExecutorService executorService = Executors.newSingleThreadScheduledExecutor();
19-
ScheduledFuture<?> timer = executorService.schedule(new Runnable() {
19+
ScheduledFuture<?> interruptTimer = executorService.schedule(new Runnable() {
2020
@Override
2121
public void run() {
2222
if (!done.get()) {
2323
executionThread.interrupt();
2424
}
2525
}
2626
}, timeoutMillis, TimeUnit.MILLISECONDS);
27+
28+
// We'll start a second timer that will stop() the thread in case interrupt()
29+
// doesn't do it. This can happen for *uninterruptible threads*.
30+
ScheduledFuture<?> stopTimer = executorService.schedule(new Runnable() {
31+
@Override
32+
public void run() {
33+
if (!done.get()) {
34+
executionThread.stop();
35+
}
36+
}
37+
}, timeoutMillis * 2, TimeUnit.MILLISECONDS);
2738
try {
2839
return callback.call();
2940
} catch (InterruptedException timeout) {
3041
throw new TimeoutException("Timed out after " + timeoutMillis + "ms.");
42+
} catch (ThreadDeath threadDeath) {
43+
throw new TimeoutException("Timed out after " + timeoutMillis + "ms. (Stopped the thread was uninterruptible).");
3144
} finally {
3245
done.set(true);
33-
timer.cancel(true);
46+
interruptTimer.cancel(true);
47+
stopTimer.cancel(true);
3448
executorService.shutdownNow();
3549
}
3650

core/src/test/java/cucumber/runtime/TimeoutTest.java

+48-32
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package cucumber.runtime;
22

3+
import org.junit.Rule;
34
import org.junit.Test;
5+
import org.junit.rules.ExpectedException;
46

57
import java.util.concurrent.CountDownLatch;
68
import java.util.concurrent.TimeoutException;
@@ -11,6 +13,9 @@
1113
import static org.junit.Assert.fail;
1214

1315
public class TimeoutTest {
16+
@Rule
17+
public ExpectedException exception = ExpectedException.none();
18+
1419
@Test
1520
public void doesnt_time_out_if_it_doesnt_take_too_long() throws Throwable {
1621
final Slow slow = new Slow();
@@ -23,42 +28,54 @@ public String call() throws Throwable {
2328
assertEquals("slept 10ms", what);
2429
}
2530

26-
@Test(expected = TimeoutException.class)
31+
@Test
2732
public void times_out_if_it_takes_too_long() throws Throwable {
28-
final Slow slow = new Slow();
29-
Timeout.timeout(new Timeout.Callback<String>() {
30-
@Override
31-
public String call() throws Throwable {
32-
return slow.slow(100);
33-
}
34-
}, 50);
35-
fail();
33+
try {
34+
final Slow slow = new Slow();
35+
Timeout.timeout(new Timeout.Callback<String>() {
36+
@Override
37+
public String call() throws Throwable {
38+
return slow.slow(100);
39+
}
40+
}, 50);
41+
fail();
42+
} catch (TimeoutException expected) {
43+
assertEquals("Timed out after 50ms.", expected.getMessage());
44+
}
3645
}
3746

38-
@Test(expected = TimeoutException.class)
39-
public void times_out_infinite_loop_if_it_takes_too_long() throws Throwable {
40-
final Slow slow = new Slow();
41-
Timeout.timeout(new Timeout.Callback<Void>() {
42-
@Override
43-
public Void call() throws Throwable {
44-
slow.infinite();
45-
return null;
46-
}
47-
}, 10);
48-
fail();
47+
@Test
48+
public void times_out_infinite_spin_loop_if_it_takes_too_long() throws Throwable {
49+
try {
50+
final Slow slow = new Slow();
51+
Timeout.timeout(new Timeout.Callback<Void>() {
52+
@Override
53+
public Void call() throws Throwable {
54+
slow.infiniteSpin();
55+
return null;
56+
}
57+
}, 10);
58+
fail();
59+
} catch (TimeoutException expected) {
60+
assertEquals("Timed out after 10ms. (Stopped the thread was uninterruptible).", expected.getMessage());
61+
}
4962
}
5063

51-
@Test(expected = TimeoutException.class)
64+
@Test
5265
public void times_out_infinite_latch_wait_if_it_takes_too_long() throws Throwable {
53-
final Slow slow = new Slow();
54-
Timeout.timeout(new Timeout.Callback<Void>() {
55-
@Override
56-
public Void call() throws Throwable {
57-
slow.infiniteLatchWait();
58-
return null;
59-
}
60-
}, 10);
61-
fail();
66+
try {
67+
final Slow slow = new Slow();
68+
Timeout.timeout(new Timeout.Callback<Void>() {
69+
@Override
70+
public Void call() throws Throwable {
71+
slow.infiniteLatchWait();
72+
return null;
73+
}
74+
}, 10);
75+
fail();
76+
} catch (TimeoutException expected) {
77+
assertEquals("Timed out after 10ms.", expected.getMessage());
78+
}
6279
}
6380

6481
@Test
@@ -93,9 +110,8 @@ public String slow(int millis) throws InterruptedException {
93110
return String.format("slept %sms", millis);
94111
}
95112

96-
public void infinite() throws InterruptedException {
113+
public void infiniteSpin() throws InterruptedException {
97114
while (true) {
98-
sleep(1);
99115
}
100116
}
101117

0 commit comments

Comments
 (0)