`TestScope.runTest` may cause thread starvation (JVM, Native)

[OliverO2]

The documentation for TestScope states.

If [context] provides a [Job], that job is used as a parent for the new scope.

This implies that TestScope is meant to be used inside an existing coroutine. In that case, runBlocking must be avoided to guard against possible thread starvation (c.f. #3983).

What happens now:

TestScope.runTest() currently calls createTestResult(), which translates into runBlocking on JVM and Native.

What should happen instead:

Only the top-level runTest functions should be allowed to call runBlocking. TestScope.runTest() should avoid calls to runBlocking.

Use case:

The TestBalloon test framework organizes test suites in a multi-level hierarchy, with a coroutine hierarchy mirroring the suite hierarchy. It uses TestScope.runTest to run individual tests at the lowest levels of that hierarchy, with virtual time and exception handling. TestBalloon can run tests in parallel using Dispatchers.Default. (Note: TestBalloon has its own mechanism to provide the necessary Promise to the JS-based infrastructure independent of TestScope.runTest().)

Users of TestBalloon have experienced thread starvation with parallel test invocation in conjunction with TestScope. Disabling TestScope did not show thread starvation. If a reproducer is required, we could work to provide one.

[dkhalanskyjb]

Hi!

This implies that TestScope is meant to be used inside an existing coroutine.

Nope, this entire premise is flawed. runTest is intended to be the root coroutine running in a test. We should update the documentation to clearly state that. Right now, it only says that it's similar in behavior to runBlocking, which may be too implicit.

Job instances do not necessarily correspond to coroutines. You could create a Job() or a CompletableDeferred() manually and pass that as an argument. A plausible reason for why you'd want to do that doesn't come to mind, and truth be told, we only added that behavior for uniformity between TestScope() and CoroutineScope(), without a specific intended use case. Another option would be to throw an exception if a Job was passed, but we didn't think of a way someone could have misused that functionality accidentally, so we left that in. Thank you for providing an example of a problematic behavior!

A reproducer for the problematic behavior is not necessary. Calling runBlocking inside Dispatchers.Default is known to be incorrect.

Only the top-level runTest functions should be allowed to call runBlocking. TestScope.runTest() should avoid calls to runBlocking.

The way you are describing this, it's not possible to implement without making runTest a suspend function. Since runTest has to wait for asynchronous computations in the spawned coroutines to finish—including cases when another thread is used—it needs to either block (hence runBlocking) or suspend while an indefinite amount of time passes.

We can consider making a separate suspend fun testScope(block: TestScope.() -> Unit), similar to coroutineScope or supervisorScope, that would run its block the way runTest runs a test, but runTest behaves as intended. Would you be interested in testScope?

[OliverO2]

Job instances do not necessarily correspond to coroutines.

The 1:1 correspondence was always part of my mental model. The distinction seemed to be an implementation detail.

Another option would be to throw an exception if a Job was passed

I'd view this as preferable it there is no constructive use case. In contrast to documentation, it would prevent users from accidentally launching a footgun that's difficult to debug.

We can consider making a separate suspend fun testScope(block: TestScope.() -> Unit), similar to coroutineScope or supervisorScope, that would run its block the way runTest runs a test, but runTest behaves as intended. Would you be interested in testScope?

That's an excellent idea and I'd much appreciate it.

Use case: With larger numbers of sufficiently isolated tests, running them in parallel on Dispatchers.Default has been demonstrated to show very high CPU utilization and greatly reduced overall runtimes for tests, like down from 4:53 to 1:19 minutes for one test series. This deep concurrency made kotlinx.coroutines shine when compared to conventional coarse-grained parallelism on upper invocation levels. And the test dispatcher's virtual time provides an ideal complement for parallel execution as real time makes less sense on a highly loaded system.

So having a separate suspend fun testScope(block: TestScope.() -> Unit) would support this use case with less coupling due to a smaller API surface, compared to re-implementing virtual time outside kotlinx.coroutines.

Am I assuming correctly that if I'd provide a TestDispatcher or its scheduler to that testScope function as part of its coroutineContext, we must make sure that such TestDispatcher (or its scheduler) cannot be shared across multiple testScope functions running in parallel?

[dkhalanskyjb]

The distinction seemed to be an implementation detail.

The simplest way to observe the distinction is to manually create a coroutine scope using the CoroutineScope() constructor function. It's not a coroutine, but it has a Job, which can be used to cancel the whole collection of coroutines launched in that scope. So, Job is a general mechanism for constructing lifetime hierarchies in kotlinx.coroutines.

Am I assuming correctly that if I'd provide a TestDispatcher or its scheduler to that testScope function

That is a difficult question.

In the ideal world, the entirety of the test forms a single structured concurrency hierarchy, with TestScope at the top and every child Job descending from it. However, people pass test dispatchers to the systems under test without making a child scope to inject: https://grep.app/search?f.lang=Kotlin&q=%3A+CoroutineDispatcher. Then, the only indicator that some work created by the test hasn't completed is a task in a test dispatcher.

(While writing this, I realized that our test framework is dealing with that situation worse than it could be: #4580)

If a test scheduler can be shared between tests, then tests can no longer rely on the dispatcher being empty. This also can't be solved by doing testScope { body() }; testScheduler.advanceUntilIdle(), as test scheduler tasks may want to create new child coroutines for TestScope. It would be very strange to prohibit them from doing so, as they are still part of the test, the test hasn't logically completed by that time.

To sum it up, the only way we can support dispatcher injection in systems under test without creating child scopes of TestScope is to make each test responsible for its own coroutine scheduler.

That said, testScope may be explicitly implemented as a building block for third-party test frameworks, in which case, other behaviors are possible if the test framework is stricter about structured concurrency. Do you think it would be viable to prohibit TestBalloon users from injecting dispatchers into coroutines that are not children of the TestScope?

[OliverO2]

So, Job is a general mechanism for constructing lifetime hierarchies in kotlinx.coroutines.

Understood. Could that be simplified by dropping the distinction between Job and coroutine, and make users create a coroutine per lifetime hierarchy level instead?

Do you think it would be viable to prohibit TestBalloon users from injecting dispatchers into coroutines that are not children of the TestScope?

Yes, if I change the execution model: Currently, a TestScope is created per test. Tests form the lowest level of the test element hierarchy, which mirrors a coroutine hierarchy. Tests can request access to shared state via fixtures. Such fixtures can be created at any (test suite) level in the test element hierarchy, have a lifetime of that level's execution, and execute within that level's coroutine.

Now what I could do is drop the ability to create a TestSope per test, and restrict it to the compartment level. Test compartments form the second level of the execution hierarchy, and always run in isolation. So I could guarantee that all tests (and the fixtures they use) either run in the same TestScope or they don't have a TestScope at all and won't get one (unless they'd explicitly call kotlinx.coroutines functions to do so).

Would that make sense?

Could I allow parallelism under such a TestScope and still share virtual time across all coroutines executing in that TestScope?

[dkhalanskyjb]

Could I allow parallelism under such a TestScope and still share virtual time across all coroutines executing in that TestScope?

Then, if a test launches some computations outside structured concurrency, the test will have no way of knowing some computations are still pending, which will lead to test cross-contamination. I.e., one test launches something, and some other test fails because it observes a failure. This greatly confuses the users, which we observed in a mild form because of uncaught exception handlers being installed globally: https://github.com/Kotlin/kotlinx.coroutines/issues?q=uncaughtexceptionsbeforetest

[OliverO2]

Then, if a test launches some computations outside structured concurrency

Isn't that almost always a recipe for a planned disaster which we cannot avoid if someone insists?

But I'd expect that to be way less pronounced with TestBalloon. Its scope-friendly architecture encourages users (by making it easy) to stay within structured concurrency at all times:

• If users need to run services, they can easily do so with fixtures, which automatically close and their coroutines get cancelled at the end of their lifetime.
• If users need to use a Main dispatcher, they'd put them into a safe compartment for it, or use TestConfig.mainDispatcher().
• Replacing the classic static test setup (including @Before and @After) with a properly scoped structure avoids typical footguns when dealing with structured concurrency and other scope-oriented designs.

Having said that, there is a responsibility to design a test setup in a reasonable way. That responsibility ultimately rests with the users. Even more so if tests run concurrently. We can make things as simple as possible, but not simpler.

So it sounds like:

1. The proposed suspend fun testScope(block: TestScope.() -> Unit) would be a viable way to safely provide a TestScope to a number of tests running concurrently on Dispatchers.Default.
2. If the TestDispatcher could still handle virtual time in the presence of parallel execution, that would be ideal. (If not, I could restrict that to single-threaded execution.)

If that's OK with you, I'd much appreciate such a testScope() implementation coming to kotlinx.coroutines and would be happy to integrate it into TestBalloon.