fix: rework reentrant concurrency model for Java interop

[EugeneTheDev]

Problem

For more stable Java interop, we need to properly handle the reentrant scenario Suspendable Koog code → Blocking user Java code → Suspendable Koog code. Naively wrapping Java-interacting components in runBlocking(context) or withContext(context) introduces a risk of deadlock or thread starvation.

Consider the following example:

val singleThreadDispatcher = Executors.newSingleThreadExecutor().asCoroutineDispatcher()

fun suspendableKotlinCodeOne() {
  runBlocking(singleThreadDispatcher) {
    blockingJavaCodeTwo()
  }
}

fun suspendableKotlinCodeThree() {
  runBlocking(singleThreadDispatcher) {
    // do something
  }
}

public void blockingJavaCodeTwo() {
  suspendableKotlinCodeThree();
}

suspendableKotlinCodeOne dispatches a block onto singleThreadDispatcher, which then calls into blockingJavaCodeTwo. The Java method calls back into Kotlin via suspendableKotlinCodeThree, which again tries to dispatch onto the same single-threaded executor.

Even though execution stays on the same thread throughout, the coroutine context tracking this is lost when crossing the Kotlin → Java → Kotlin boundary. As a result, suspendableKotlinCodeThree cannot tell that it is already running on the correct context and attempts to redispatch — causing a deadlock, since the only thread is already occupied. The redispatch was never actually necessary; the inner block could have executed directly in place.

Current state

The solution is to store the coroutine context in a thread local, then check it on dispatch to determine whether the requested context is already active. Several previous attempts have tried to implement this and fix related bugs, the most recent being #1716. However, the current implementation still has major problems:

• After the last fix, runBlockingIfRequired ignores its context argument, meaning it never dispatches to the requested dispatcher.
• Thread-local-based dispatcher tracking stores the context on the wrong thread — the current thread running runBlockingIfRequired, rather than the thread associated with the supplied context.
• There is a bunch of similar-looking helper functions implemented separately, leading to duplication. The naming is also confusing.

Solution

I've narrowed the scope down to two functions with (hopefully) clearer names: runBlockingReentrant and withContextReentrant. They implement the approach described above correctly, avoiding unnecessary dispatch. A few tests have been added to verify the behavior. All existing utility functions were deleted and all implementations have been migrated.

Important note

The case where a user manually submits a task to the executor outside of Koog — for example, running the agent itself on the same single-threaded executor — is deliberately out of scope. In such cases, it is technically impossible to reliably determine whether we are already on the same executor/thread. This should be treated as user error rather than something Koog tries to handle.

DEPRECATED:

• Constructors and methods in AIAgentConfig on the JVM that accept ExecutorService parameters. Constructors and methods that accept more generic Executor should be used instead.

closes KG-750

[sdubov]

@EugeneTheDev , nice work! Thank you for the changes. I've left several general related comments.

[sdubov]

Why did you delete the return part of KDoc?

[EugeneTheDev]

They seemed too verbose, without any actual value. You can see the return type yourself in your IDE, so that part only bloats the KDoc, IMHO, because it doesn't provide any additional valuable info on the returned value

[sdubov]

Would not it be better to annotate the exact internal call instead of a whole function?

[EugeneTheDev]

I would say that's gonna be too granular. Prefering method-level opt-ins to class level opt-ins is fine, I agree here. But going ever more granular might not be the best idea from the readability side of things. I don't really have any strong opinion here, but putting opt-in on the method level is already an established pattern in the whole project, so I'm just following it

[sdubov]

strategy or llm request dispatcher?

[EugeneTheDev]

Good catch, will fix

[sdubov]

The context from the executorService is not used here. Is it by design?

[EugeneTheDev]

Good catch, will fix

[sdubov]

Why do these dispatchers lives in the AIAgentConfig? I would assume that they should be located closer to the executor, instead of configurator. What do you think?

[EugeneTheDev]

That's the only central configuration object we currently have. So it's the only option to put global configurations that should affect the whole agent

[sdubov]

Previously in the CoroutineUtils we had thow helper methods: runOnLLMDispatcher and runOnStrategyDispatcher`. It looks like a handy shortcut to delegate execution on a correct thread instead of writing:

runBlockingReentrant(dispatcher ?: config.strategyDispatcher) { ... }

Maybe it worth thinking about returning them?

TBD: @EugeneTheDev , also I have some concern that we currently have a contract for using suspend Kotlin calls from Java API that seems easy to break. Maybe we should expose two API for such cases and hide the rest from a public API, limiting the probability of mistakes. What do you think?

[EugeneTheDev]

Maybe it worth thinking about returning them?

Initially, I thought that this inline approach would be more readable, but I guess you're right, and these shorthands did simplify things a bit. I'll return them back

we currently have a contract for using suspend Kotlin calls from Java API that seems easy to break

Can you clarify, please, wdym? I don't quite get it

[sdubov]

Not related to a current change (feel free to skip this comment), but I often see OptIn on file level in our code base. @EugeneTheDev, do you consider this a good practice? I would add annotation for a particular invocation statement instead.

[EugeneTheDev]

Oh, that's a good comment. I actually don't like file-level opt-ins or suppresions unless there's a really good reason (like you need to opt in on the same API that is used by several top-level functions in this file). But, reiterating on my comment above, I think that statement-level opt-in might be too granular. IMHO method/function level is a sweet spot