Spring New threads added to thread pool inherit active database transaction in tests

TransactionalTestExecutionListener's sanity check for open transactions can cause spurious test failures when JUnit tests are run concurrently on code that uses ForkJoinPool.managedBlock().

One possible fix would be to skip the sanity check the first time a test runs on a particular thread.

More details:

When concurrent test execution is enabled in JUnit 5, it creates a ForkJoinPool to run the tests.

TransactionalTestExecutionListener has a sanity check that runs before each test method to make sure that the current thread doesn't have a transaction left over from a previous test that failed abnormally. If it finds one, it throws an exception and the test fails with

java.lang.IllegalStateException: Cannot start new transaction without ending existing transaction" in TransactionalTestExecutionListener

The thread-local variable in TransactionContextHolder that holds the current transaction is inheritable, meaning that any child threads created from inside a test method will inherit the open transaction from the parent thread. This is correct in cases where test threads are spawning short-lived temporary threads. But it does the wrong thing if a test calls code that adds a thread to the current thread pool.

One such method is ForkJoinPool.managedBlock(), whose contract says that if it's called from a thread that's managed by a ForkJoinPool, and the pool has no available threads, it will add a new thread to the pool to ensure there's spare capacity.

When that happens during a concurrent JUnit run, the newly-added spare thread (which, since it was spawned from a thread with an open transaction, inherits the transaction context) immediately starts working on the next test in the pool's task queue. The sanity check in TransactionalTestExecutionListener sees the inherited transaction context and bombs out. The end user sees tests failing seemingly at random.

In my application, I worked around this by adding a listener that runs before TransactionalTestExecutionListener and clears the transaction context the first time it executes on a given thread. Kotlin code:

package org.springframework.test.context.transaction

import org.springframework.core.annotation.Order
import org.springframework.test.context.TestContext
import org.springframework.test.context.TestExecutionListener

@Order(3000) // TransactionalTestExecutionListener is order 4000
class InheritedTransactionRemover : TestExecutionListener {
  private val transactionRemoved = ThreadLocal.withInitial { false }

  override fun beforeTestMethod(testContext: TestContext) {
    if (!transactionRemoved.get()) {
      TransactionContextHolder.removeCurrentTransactionContext()
      transactionRemoved.set(true)
    }
  }
}

This fixes the random test failures for me. I believe the same concept would probably be the right fix for the problem in general: skip the sanity check the first time a given thread is used to run tests. There can't be a leftover transaction from an earlier test in that situation.

Note that this class has to go in the org.springframework.test.context.transaction package because TransactionContextHolder is package-private. I'm not using JPMS (modules) so I'm not sure if additional steps would be needed to make it work with modules.

Comment From: encircled

I was able to reproduce this issue. Thanks, @sgrimm, for pointing out the use of ForkJoinPool.managedBlock().

Here’s a shortened description of the problem: - JUnit 5 runs in parallel mode, which uses ForkJoinPool for test execution. - A user’s test body invokes ForkJoinPool.managedBlock(...), which may spawn a new thread. This thread inherits the ThreadLocal currentTransactionContext and is added back to the pool. - When this thread is later picked up by another transactional test, it fails due to an assertion on transaction existence. - Unfortunately, there is no reliable way to determine whether a thread was spawned in this way. An indirect marker might be if the worker thread index exceeds the pool size, for example, ForkJoinPool-1-worker-5 with a poolSize of 4.

Additionally, I’m not sure whether this assertion is necessary at all. What exactly are we trying to prevent here? Maybe it could be converted into a log warning instead.

Any thoughts, @sbrannen?

Comment From: kajh

Thank you so much for posting this workaround!

I have tried to do this in java, but it seems that the classes TransactionContextHolder and TransactionContext are not public in Spring 6.x or 5.x. Thank you in advance if anyone has any ideas to how to work around that!

Comment From: sgrimm

It seems I left out one important detail: you need to put InheritedTransactionRemover in the org.springframework.test.context.transaction package so it can access TransactionContextHolder which is package-private. I've edited my earlier comment to mention that.

Comment From: kajh

I see. Thank you, this is very helpful!