withTimeout with a long duration keeps the DefaultExecutor thread alive for much longer than necessary

[chaoren]

Describe the bug

What happened?

After withTimeout, the DefaultExecutor thread will frequently stay around for the entire length of the timeout specified even if the actual withTimeout block finishes immediately. This seems to be ignoring the "keep alive" duration configured for DefaultExecutor:

kotlinx.coroutines/kotlinx-coroutines-core/jvm/src/DefaultExecutor.kt

Lines 32 to 39 in 60d2fe8

	private const val DEFAULT_KEEP_ALIVE_MS = 1000L // in milliseconds
	private val KEEP_ALIVE_NANOS = TimeUnit.MILLISECONDS.toNanos(
	try {
	java.lang.Long.getLong("kotlinx.coroutines.DefaultExecutor.keepAlive", DEFAULT_KEEP_ALIVE_MS)
	} catch (e: SecurityException) {
	DEFAULT_KEEP_ALIVE_MS
	})

What should have happened instead?

The DefaultExecutor thread should only stay alive for at most one second after becoming idle according to DEFAULT_KEEP_ALIVE_MS.

Provide a Reproducer

Run this a bunch of times:

fun main() {
    runBlocking { withTimeout(10.seconds) {} }
    val time = measureTime {
        arrayOfNulls<Thread>(Thread.activeCount())
            .apply(Thread::enumerate)
            .find { it?.name == "kotlinx.coroutines.DefaultExecutor" }!!
            .join()
    }
    println(time)
}

Sometimes the thread takes about one second to join as expected, but more often than not, it'll take 11 seconds instead. Seems to be the Duration given to withTimeout plus one second (i.e., how long it was supposed to stay alive for) whenever that happens.

[dkhalanskyjb]

Yes, that's correct, once DefaultExecutor sees that it's supposed to run some code eventually, it will just go to sleep and won't wake up until either the time comes to run that code, or some new event arrives. KEEP_ALIVE_MS is not part of the public contract and isn't advertised anywhere, so it's just a part of our internal implementation, not something you can rely on.

If you have specific requirements for when DefaultExecutor should finish, please share them. Without specific use cases, we'd rather not wake DefaultExecutor up each time a task is removed from its queue: let it sleep in peace.

Also, if you are okay with relying on the internal implementation details, you can interrupt the DefaultExecutor so that it recognizes that it doesn't have any more work to do:

runBlocking { withTimeout(10.seconds) {} }
val time = measureTime {
    arrayOfNulls<Thread>(Thread.activeCount())
        .apply(Thread::enumerate)
        .find { it?.name == "kotlinx.coroutines.DefaultExecutor" }!!
        .apply {
            interrupt()
            join()
        }
}
println(time)

[chaoren]

This came up because 9a98eab changed timeout of runTest to 60 seconds, which caused the DefaultExecutor thread to far outlive the lifetime of each test case and it was caught by a leaked thread detector that some of our tests used. That thread should obviously have been exempt from the detector anyway so that wasn't a real problem, but I thought that maybe this behavior was unintentional after seeing that the thread was only supposed to live for one second.

we'd rather not wake DefaultExecutor up each time a task is removed from its queue: let it sleep in peace.

Fair enough. I'll just consider this an implementation quirk then. No real problems here.