Skip to content

Commit e554455

Browse files
sherryfoxcopybara-github
authored andcommitted
fix: AdkWebServer.stop() when started with wait = true
Previously, starting the server with wait = true blocked the thread before the server field was assigned. This prevented stop() from working because the server reference remained null. This change assigns the server field before starting the engine, and adds a try-catch block to reset it if startup fails. A new lifecycle test is added to verify this behavior. PiperOrigin-RevId: 967034991
1 parent 998a002 commit e554455

2 files changed

Lines changed: 191 additions & 17 deletions

File tree

webserver/src/jvmMain/kotlin/com/google/adk/kt/webserver/AdkWebServer.kt

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@ import org.slf4j.event.Level
5656
/**
5757
* Embedded Ktor server exposing the ADK dev/web API.
5858
*
59+
* [start] and [stop] are safe to call from different threads; a [stop] arriving while [start] is
60+
* still binding aborts it. A failed [start] leaves the engine recorded, so call [stop] before
61+
* retrying.
62+
*
5963
* @property captureMessageContent When true, the server records prompt/response content into
6064
* telemetry spans so the Dev UI trace view can display it. This may capture PII and increase span
6165
* size, so it defaults to false; enable it only for local development.
@@ -76,29 +80,35 @@ class AdkWebServer(
7680
fun adkVersion(): String = com.google.adk.kt.VERSION
7781
}
7882

83+
private val lifecycleLock = Any()
7984
private var server: ApplicationEngine? = null
8085

8186
fun start(wait: Boolean = false) {
82-
if (server != null) return
83-
84-
server =
85-
embeddedServer(Netty, port = port) {
86-
adkModule(
87-
sessionService,
88-
artifactService,
89-
agentLoader,
90-
apiServerSpanExporter,
91-
captureMessageContent,
92-
plugins,
93-
)
94-
}
95-
.start(wait = wait)
96-
logger.info("Ktor server started on port $port")
87+
// Released before the blocking call below, so stop() can still take it.
88+
val engine =
89+
synchronized(lifecycleLock) {
90+
if (server != null) return
91+
embeddedServer(Netty, port = port) {
92+
adkModule(
93+
sessionService,
94+
artifactService,
95+
agentLoader,
96+
apiServerSpanExporter,
97+
captureMessageContent,
98+
plugins,
99+
)
100+
}
101+
.also { server = it }
102+
}
103+
logger.info("Ktor server starting on port $port")
104+
engine.start(wait = wait)
97105
}
98106

99107
fun stop() {
100-
server?.stop(1000, 5000)
101-
server = null
108+
synchronized(lifecycleLock) {
109+
server?.stop(1000, 5000)
110+
server = null
111+
}
102112
logger.info("Ktor server stopped")
103113
}
104114

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
/*
2+
* Copyright 2026 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.adk.kt.webserver
18+
19+
import com.google.adk.kt.webserver.telemetry.ApiServerSpanExporter
20+
import com.google.common.truth.Truth.assertThat
21+
import java.io.IOException
22+
import java.net.HttpURLConnection
23+
import java.net.ServerSocket
24+
import java.net.URL
25+
import java.util.Collections
26+
import java.util.concurrent.CyclicBarrier
27+
import java.util.concurrent.TimeUnit
28+
import kotlin.concurrent.thread
29+
import org.junit.Assert.assertThrows
30+
import org.junit.Test
31+
import org.junit.runner.RunWith
32+
import org.junit.runners.JUnit4
33+
34+
/** Start and stop against a real socket, which `testApplication` based tests cannot reach. */
35+
@RunWith(JUnit4::class)
36+
class AdkWebServerLifecycleTest {
37+
38+
@Test
39+
fun stop_shutsDownAServerStartedWithWait() {
40+
val port = freePort()
41+
val server = newServer(port)
42+
val serverThread =
43+
thread(name = "adk-webserver-lifecycle-test", isDaemon = true) { server.start(wait = true) }
44+
45+
try {
46+
awaitHealthy(port)
47+
48+
server.stop()
49+
50+
// start(wait = true) returns only after shutdown, so a live thread means stop() did nothing.
51+
serverThread.join(SHUTDOWN_TIMEOUT_MILLIS)
52+
assertThat(serverThread.isAlive).isFalse()
53+
} finally {
54+
server.stop()
55+
}
56+
}
57+
58+
@Test
59+
fun concurrentStarts_startASingleServer() {
60+
val port = freePort()
61+
val server = newServer(port)
62+
// A barrier, not a latch: every racer must be parked before any of them calls start().
63+
val startLine = CyclicBarrier(RACING_THREADS)
64+
val failures = Collections.synchronizedList(mutableListOf<Throwable>())
65+
val racers =
66+
(1..RACING_THREADS).map {
67+
thread(name = "adk-webserver-start-race-$it", isDaemon = true) {
68+
try {
69+
startLine.await(STARTUP_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS)
70+
server.start()
71+
} catch (t: Throwable) {
72+
failures.add(t)
73+
}
74+
}
75+
}
76+
77+
try {
78+
racers.forEach {
79+
it.join(STARTUP_TIMEOUT_MILLIS)
80+
assertThat(it.isAlive).isFalse()
81+
}
82+
83+
// Losing the race must be a no-op, not a second engine fighting for the same port.
84+
assertThat(failures).isEmpty()
85+
awaitHealthy(port)
86+
} finally {
87+
server.stop()
88+
}
89+
90+
assertThat(portIsFree(port)).isTrue()
91+
}
92+
93+
@Test
94+
fun startAfterAFailedStart_needsStopFirst() {
95+
val port = freePort()
96+
val server = newServer(port)
97+
98+
ServerSocket(port).use { assertThrows(Exception::class.java) { server.start() } }
99+
100+
// The failed engine stays recorded, so retrying without stop() hits the already-started
101+
// guard and binds nothing, even though the port is free again.
102+
server.start()
103+
assertThat(portIsFree(port)).isTrue()
104+
105+
server.stop()
106+
try {
107+
server.start()
108+
awaitHealthy(port)
109+
} finally {
110+
server.stop()
111+
}
112+
}
113+
114+
private fun newServer(port: Int) =
115+
AdkWebServer(
116+
port = port,
117+
sessionService = FakeSessionService(),
118+
artifactService = FakeArtifactService(),
119+
agentLoader = FakeAgentLoader(),
120+
apiServerSpanExporter = ApiServerSpanExporter(),
121+
)
122+
123+
private fun portIsFree(port: Int): Boolean =
124+
try {
125+
ServerSocket(port).close()
126+
true
127+
} catch (_: IOException) {
128+
false
129+
}
130+
131+
private fun awaitHealthy(port: Int) {
132+
val deadline = System.nanoTime() + STARTUP_TIMEOUT_MILLIS * NANOS_PER_MILLI
133+
while (System.nanoTime() < deadline) {
134+
if (healthStatusOrNull(port) == HttpURLConnection.HTTP_OK) return
135+
Thread.sleep(POLL_INTERVAL_MILLIS)
136+
}
137+
throw AssertionError("Server did not serve /health within $STARTUP_TIMEOUT_MILLIS ms")
138+
}
139+
140+
private fun healthStatusOrNull(port: Int): Int? =
141+
try {
142+
val connection = URL("http://127.0.0.1:$port/health").openConnection() as HttpURLConnection
143+
connection.connectTimeout = CONNECT_TIMEOUT_MILLIS
144+
connection.readTimeout = CONNECT_TIMEOUT_MILLIS
145+
try {
146+
connection.responseCode
147+
} finally {
148+
connection.disconnect()
149+
}
150+
} catch (_: IOException) {
151+
null
152+
}
153+
154+
private fun freePort(): Int = ServerSocket(0).use { it.localPort }
155+
156+
private companion object {
157+
const val STARTUP_TIMEOUT_MILLIS = 20_000L
158+
const val SHUTDOWN_TIMEOUT_MILLIS = 10_000L
159+
const val POLL_INTERVAL_MILLIS = 50L
160+
const val CONNECT_TIMEOUT_MILLIS = 1_000
161+
const val RACING_THREADS = 8
162+
const val NANOS_PER_MILLI = 1_000_000L
163+
}
164+
}

0 commit comments

Comments
 (0)