Commit 81e7884
authored
fix: rework reentrant concurrency model for Java interop (#1945)
## 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:
```kotlin
val singleThreadDispatcher = Executors.newSingleThreadExecutor().asCoroutineDispatcher()
fun suspendableKotlinCodeOne() {
runBlocking(singleThreadDispatcher) {
blockingJavaCodeTwo()
}
}
fun suspendableKotlinCodeThree() {
runBlocking(singleThreadDispatcher) {
// do something
}
}
```
```java
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](https://youtrack.jetbrains.com/issue/KG-750)1 parent cc3ebc5 commit 81e7884
39 files changed
Lines changed: 649 additions & 706 deletions
File tree
- agents
- agents-core/src
- jvmCommonMain/kotlin/ai/koog/agents
- core
- agent
- config
- context
- entity
- feature/pipeline
- session
- dsl/builder
- feature
- message
- pipeline
- utils
- planner/goap
- jvmTest
- java/ai/koog/agents/core/agent
- kotlin/ai/koog/agents/core/agent
- agents-features
- agents-features-event-handler/src/jvmCommonMain/kotlin/ai/koog/agents/features/eventHandler/feature
- agents-features-snapshot/src
- commonMain/kotlin/ai/koog/agents/snapshot/feature
- jvmCommonMain/kotlin/ai/koog/agents/snapshot/feature
- convention-plugin-ai/src/main/kotlin/ai/koog/gradle/fixups
- integration-tests/src/jvmTest/java/ai/koog/integration/tests
- agent
- utils
- prompt
- prompt-executor
- prompt-executor-clients/src/jvmMain/kotlin/ai/koog/prompt/executor/clients
- prompt-executor-model/src/jvmMain/kotlin/ai/koog/prompt/executor/model
- prompt-model/src/jvmMain/kotlin/ai/koog/prompt/execution/utils
- utils/src
- commonMain/kotlin/ai/koog/utils/annotations
- jvmCommonMain/kotlin/ai/koog/utils/concurrency
- jvmTest/kotlin/ai/koog/utils/concurrency
Lines changed: 6 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
13 | | - | |
| 13 | + | |
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
| 19 | + | |
19 | 20 | | |
20 | 21 | | |
21 | 22 | | |
| |||
37 | 38 | | |
38 | 39 | | |
39 | 40 | | |
| 41 | + | |
40 | 42 | | |
41 | 43 | | |
42 | 44 | | |
43 | 45 | | |
44 | 46 | | |
45 | 47 | | |
46 | 48 | | |
47 | | - | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
48 | 52 | | |
49 | 53 | | |
50 | 54 | | |
| |||
Lines changed: 6 additions & 55 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | | - | |
3 | 2 | | |
4 | 3 | | |
5 | 4 | | |
| |||
8 | 7 | | |
9 | 8 | | |
10 | 9 | | |
11 | | - | |
| 10 | + | |
12 | 11 | | |
13 | 12 | | |
14 | 13 | | |
15 | 14 | | |
16 | 15 | | |
17 | 16 | | |
| 17 | + | |
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
| |||
56 | 56 | | |
57 | 57 | | |
58 | 58 | | |
59 | | - | |
| 59 | + | |
60 | 60 | | |
61 | 61 | | |
62 | 62 | | |
| |||
98 | 98 | | |
99 | 99 | | |
100 | 100 | | |
101 | | - | |
| 101 | + | |
102 | 102 | | |
103 | 103 | | |
104 | 104 | | |
| |||
114 | 114 | | |
115 | 115 | | |
116 | 116 | | |
117 | | - | |
| 117 | + | |
118 | 118 | | |
119 | 119 | | |
120 | 120 | | |
| |||
133 | 133 | | |
134 | 134 | | |
135 | 135 | | |
136 | | - | |
| 136 | + | |
137 | 137 | | |
138 | 138 | | |
139 | 139 | | |
140 | | - | |
141 | | - | |
142 | | - | |
143 | | - | |
144 | | - | |
145 | | - | |
146 | | - | |
147 | | - | |
148 | | - | |
149 | | - | |
150 | | - | |
151 | | - | |
152 | | - | |
153 | | - | |
154 | | - | |
155 | | - | |
156 | | - | |
157 | | - | |
158 | | - | |
159 | | - | |
160 | | - | |
161 | | - | |
162 | | - | |
163 | | - | |
164 | | - | |
165 | | - | |
166 | | - | |
167 | | - | |
168 | | - | |
169 | | - | |
170 | | - | |
171 | | - | |
172 | | - | |
173 | | - | |
174 | | - | |
175 | | - | |
176 | | - | |
177 | | - | |
178 | | - | |
179 | | - | |
180 | | - | |
181 | | - | |
182 | | - | |
183 | | - | |
184 | | - | |
185 | | - | |
186 | | - | |
187 | | - | |
188 | | - | |
189 | 140 | | |
190 | 141 | | |
191 | 142 | | |
| |||
0 commit comments