Junit5: Declarative timeout does not interrupt code running in infinite loop

[praenubilus]

Steps to reproduce

Having some code running infinitely, but the timeout annotation(neither the class level nor testable method level) can interrupt the execution. Instead, the program hang forever.

I was hoping it can behave like the assertTimeoutPreemptively to interrupt the running test method.

e.g. I have a class with just one single infinite running method:

public class A {

    public void infinite() {
        while (true) {
            continue;
        }
    }
}

    @Test
    @Timeout(1) // run at most 1 seconds
    void pollUntil() {
        A a = new A();
        a.infinite();
        assertEquals(1,1);
    }

Context

• Used versions: Jupiter 5.5.2

[marcphilipp]

assertTimeoutPreemptively only seems to work but actually hides the underlying problem that infinite() never checks for interrupts or calls any blocking operation that would throw an InterruptedException. So, instead of blocking indefinitely, assertTimeoutPreemptively lets the thread it spawned run forever.

@Timeout on the other hand works differently. It schedules a thread to interrupt the original thread once the timeout has expired. If the original thread does not react, it will keep running until it's eventually finished, or, as in this case, until you reboot your computer. 😉

The underlying problem is that there's no API in Java that let's you reliably terminate a thread. Thus, I don't see what we could change to support methods like infinite().

Personally, I think blocking indefinitely is better than hiding the problem by keeping the code running in the background. The latter might cause strange side effects if it changes any shared state and might even lead to the JVM running out of threads when there's many such tests.

[praenubilus]

Hell @marcphilipp ,

Thanks for the feedback. I remember someone brought up this before, when the Timeout annotation being proposed: we have lots of legacy testing code in JUnit 4 relies on the timeout rules, which works in a preemptive way (from my own observation). I totally agree with you the behavior will hide some problems under the hood. But in some scenarios, the preemptive behavior is really helpful. For example we have an online Java education service, the students' submissions are totally unpredictable and we evaluate/grades the code heavily relying on the number of pass/fail unit tests. If there is some infinite running part in the code for one test case, we have no way to give a grade because the service is keep running forever. But that can be terminated in our JUnit 4 implementation with a timeout rule configuration. As we are migrating the system to JUnit 5, this becomes a headache for us. The current work around we do is enclosing every single unit test with the assertTimeoutPreemptively. And for the potential threads running in the background issue, because we run the services in a docker container, as long as all unit tests have a pass/fail result, the container will be shutdown. So that will not be an issue.

I know the aforementioned scenario is a little bit special, but I do see someone else also have the similar requirements in their code base as we use JUNIT as a part of the business logic. The new Timeout annotation introduced in v5.5 is elegant and convenient to use except lacking the preemptive behavior we want. The workaround I am thinking about: is it possible to add some "switcher" to the annotation which may look like following:

        @Test
    @Timeout(1, PREEMPTIVE=true) // run at most 1 seconds
    void pollUntil() {
        A a = new A();
        a.infinite();
        assertEquals(1,1);
    }

Or just introducing a new annotation like @TimeoutPreemptive?

[marcphilipp]

How are you executing the tests? Maybe there's a way to add a timeout around all tests? As a workaround, you could implement your own InvocationInterceptor that wraps calls to invocation.proceed() in assertTimeoutPreemptively.

[praenubilus]

How are you executing the tests? Maybe there's a way to add a timeout around all tests? As a workaround, you could implement your own InvocationInterceptor that wraps calls to invocation.proceed() in assertTimeoutPreemptively.

@marcphilipp
In JUnit4 we write a program to invoke all tests by using JUnitCore.runClasses(TestClass) to start all tests, get and analyze the results.
In JUnit 5 we use org.junit.platform.launcher.core.LauncherFactory.

Can you be more specific about the "implement your own InvocationInterceptor"? I partially get what you mean, but I believe a genuine official solution will always be the best choice.

[marcphilipp]

I meant, instead of using @Timeout you could register your own extension that does this:

public class PreemptiveTimeoutInvocationInterceptor implements InvocationInterceptor {
    @Override
    public void interceptTestMethod(Invocation<Void> invocation, ReflectiveInvocationContext<Method> invocationContext, ExtensionContext extensionContext) {
        Assertions.assertTimeoutPreemptively(Duration.ofSeconds(1), invocation::proceed);
    }
}

[marcphilipp]

Tentatively slated for 5.6 M2 solely for the purpose of _team discussion_.

[marcphilipp]

Team decision: Since a workaround exists, we won't address this for now but wait for additional interest from the community.

[kreimendahlf]

I am caught with the same use-case as @praenubilus : migrating grading scripts from JUnit 4 -> 5, and running into infinite loops that aren't timed out.

I would also be interested in a preemptive timeout that matches the language for non-preemptive.

[codejayant]

I am working on a similar use case where I would like to run a condition infinitely until the right condition matches or timeout occur which should stop the execution. Will be good if there is an option to stop the execution on timeout in @Timeout annotation.

[marcphilipp]

Just to be clear: @Timeout is preemptive. It interrupts the thread that's running the test when it times out. However, if that thread does not react to being interrupted, there's no way to stop it. assertTimeoutPreemptively does the same thing, however, it executes the supplied Executable in a different thread. If that thread does not respond to interrupts, it will continue to run, even after assertTimeoutPreemptively returns.

[praenubilus]

Just to be clear: @Timeout is preemptive. It interrupts the thread that's running the test when it times out. However, if that thread does not react to being interrupted, there's no way to stop it. assertTimeoutPreemptively does the same thing, however, it executes the supplied Executable in a different thread. If that thread does not respond to interrupts, it will continue to run, even after assertTimeoutPreemptively returns.

So can we have an annotation have the same behavior as assertTimeoutPreemptively ? Actually that's what I proposed in October last year, but the team decides not to include it at that time.

[marcphilipp]

I'd be okay with making this configurable as long as the current behavior stays the default.

[praenubilus]

I'd be okay with making this configurable as long as the current behavior stays the default.

Great. This is a long time expected feature so we can finally align and migrate all JUnit 4 test cases. Will it look like any one of following annotations?

@Timeout(1, PREEMPTIVE=true) // default is false 

@Timeout(1, FORCEQUIT=true) // default is false

@Timeout(1, UNSAFE=true)  // default is false

@TimeoutPreemptive(1)  // behave like assertTimeoutPreemptively

@TimeoutForceQuit(1)  // behave like assertTimeoutPreemptively

@TimeoutUnsafe(1)  // behave like assertTimeoutPreemptively

[marcphilipp]

Naming things is always hard. I was thinking about naming it more along the lines as to in which thread the test is going to be executed. That would yield sth. like "same thread" for the current behavior and "separate thread" for the new behavior. But I'm not really happy with those either.

@junit-team/junit-lambda Any ideas?

We also need a configuration parameter as a default timeout can be configured via configuration parameters as well. Maybe it would even be easier to start with just that and not add new annotation or attribute.

[marcphilipp]

Team decision: Let's add an Enum-valued mode attribute to @Timeout with values INFERRED (default), SAME_THREAD, and SEPARATE_THREAD along with a junit.jupiter.execution.timeout.mode.default configuration parameter to configure the default.

[christianhujer]

The underlying problem is that there's no API in Java that let's you reliably terminate a thread.

That depends on the definition of "reliably". The Thread.stop() is reliable in the sense that it will reliably stop a Thread. It's also "unsafe" as this may leave objects in an inconsistent state, which is the reason why it is deprecated (but not scheduled for removal). In the context of having a reliable timeout for tests, this may be exactly what some users (including me) might want.

https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/lang/Thread.html#stop()

Here's my test problem, simplified for reproduction:

import java.net.ServerSocket
import java.time.Duration

class SomeTest {
    @Timeout(1)
    @Test
    fun testSleep() {
        // works as expected.
        Thread.sleep(2000)
    }

    @Timeout(1)
    @Test
    fun testAccept() {
        // never stops
        ServerSocket(0).accept()
    }

    @Test
    fun testAccept2() {
        // times out as expected.
        // However, the code that called ServerSocket(0).accept() remains active and blocked.
        // This may be acceptable in many cases.
        assertTimeoutPreemptively(Duration.ofSeconds(1)) {
            ServerSocket(0).accept()
        }
}

It would be great to be able to:

• Have a declarative way of preemptive timeouts: @Timeout(mode=PREEMPT) should declaratively lead to the same behavior as assertTimeoutPreemptively().
• Have a way to use Thread.stop() for stopping the execution: @Timeout(mode=UNSAFE).

Note: I have not done any comparison with JUnit 4. I'm just adding my 2 cents, as I have run in the same issue today that @Timeout() only works on code that reacts to Thread.interrupt(). It would be very helpful, as long as there is no implementation of other declarative timeouts, that the API documentation of @Timeout mentions that this currently works only for code that reacts to Thread.interrupt().

The documentation should probably also mention that UNSAFE timeouts should be avoided. Preemptive timeouts and proper cleanup code should in most cases be completely sufficient, and will automatically lead to better and cleaner production code designs.

[dkandalov]

Here is an example in which @Timeout does fail (on a "performance bug") but only after 30 seconds instead of 1 second as expected:

class Tests {
    @Timeout(1)
    @Test fun `large input`() {
        val random = Random(seed = 42)
        val ints = generateSequence { random.nextInt(-105, 106) }.take(3000).toList()

        val triplets = ints.findZeroSumTriplets()
        assertThat(triplets.size, equalTo(303))
    }
}

fun List<Int>.findZeroSumTriplets(): Set<Triplet> {
    val result = HashSet<Triplet>()
    0.rangeTo(lastIndex - 2).forEach { i1 ->
        (i1 + 1).rangeTo(lastIndex - 1).forEach { i2 ->
            (i2 + 1).rangeTo(lastIndex).forEach { i3 ->
                if (this[i1] + this[i2] + this[i3] == 0) {
                    result.add(Triplet(this[i1], this[i2], this[i3]))
                }
            }
        }
    }
    return result
}

data class Triplet(val value: List<Int>) {
    constructor(first: Int, second: Int, third: Int): this(listOf(first, second, third).sorted())
    init {
        require(value.size == 3 && value == value.sorted())
    }
}