Skip to content

Commit 62f85a6

Browse files
committed
fix: release the JDK exchange when async dispatch wiring fails
JdkHttpTransport.executeAsync guards the whole post-adaptation dispatch path so a synchronous throw becomes a failed future. But if sendAsync had already returned a live in-flight exchange when a later step threw, that exchange kept running on a future nothing would await, leaking its connection. Hoist the in-flight handle out of the try and cancel it from the catch so the exchange is aborted on the failure path; the cancel is a no-op when the throw happened at or before dispatch (handle still null). Also document on the test's HttpClient double why its sslContext() and send() stubs are inert: executeAsync only ever calls sendAsync, so neither is reached on the path under test.
1 parent 2b6be9c commit 62f85a6

2 files changed

Lines changed: 17 additions & 2 deletions

File tree

sdk-transport-jdkhttp/src/main/kotlin/org/dexpace/sdk/transport/jdkhttp/JdkHttpTransport.kt

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,10 @@ public class JdkHttpTransport private constructor(
138138
* completions to release the body's connection back to the pool.
139139
*/
140140
override fun executeAsync(request: Request): CompletableFuture<Response> {
141+
// Held outside the try so the catch can release the JDK exchange if dispatch succeeded but a
142+
// later step threw (see the catch). Null until `sendAsync` returns: a throw at or before
143+
// dispatch leaves nothing to clean up.
144+
var inFlight: CompletableFuture<HttpResponse<InputStream>>? = null
141145
return try {
142146
val jdkRequest = requestAdapter.adapt(request, responseTimeout)
143147
// `sendAsync` is inside the guard too. Its contract does not promise that every failure
@@ -148,7 +152,7 @@ public class JdkHttpTransport private constructor(
148152
// intact whichever way a failure surfaces. (Today's stock JDK client packages such
149153
// failures into an already-failed future — e.g. on a closed client — which the bridge
150154
// propagates and so never reaches this catch; the guard is for the throwing case.)
151-
val inFlight = client.sendAsync(jdkRequest, HttpResponse.BodyHandlers.ofInputStream())
155+
inFlight = client.sendAsync(jdkRequest, HttpResponse.BodyHandlers.ofInputStream())
152156
bridgeAsyncResponse(inFlight) { jdkResponse -> responseAdapter.adapt(request, jdkResponse) }
153157
} catch (e: Exception) {
154158
// The async contract is that errors arrive through the returned future. The dispatch
@@ -160,6 +164,13 @@ public class JdkHttpTransport private constructor(
160164
// keeps a future adapter step's checked exception on the future too. Only `Error` (OOM
161165
// and other JVM-fatal conditions) is left to propagate up the caller's stack rather
162166
// than be packaged into a future that may never be awaited.
167+
//
168+
// If `sendAsync` already returned an in-flight exchange, the only way control reaches
169+
// here is `bridgeAsyncResponse` throwing before it wired that future's cancellation
170+
// propagation — its result is never returned, so cancel the exchange directly to release
171+
// its connection rather than leak it on a future nothing will await. The cancel is a
172+
// no-op when `inFlight` is null (the throw happened at or before dispatch).
173+
inFlight?.cancel(true)
163174
CompletableFuture.failedFuture<Response>(e)
164175
}
165176
}

sdk-transport-jdkhttp/src/test/kotlin/org/dexpace/sdk/transport/jdkhttp/JdkHttpTransportTest.kt

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -888,7 +888,11 @@ class JdkHttpTransportTest {
888888
* A minimal [HttpClient] whose async dispatch throws synchronously on the caller's thread,
889889
* modelling a client (or a future JDK) that does not package every `sendAsync` failure into the
890890
* returned future. Only [sendAsync] is exercised by [JdkHttpTransport.executeAsync]; the rest
891-
* are inert stubs that are never invoked on the dispatch path under test.
891+
* are inert stubs that are never invoked on the dispatch path under test. In particular
892+
* [sslContext] returns `SSLContext.getDefault()` only to satisfy the abstract member — its
893+
* checked `NoSuchAlgorithmException` and the cost of materialising the JVM default SSL context
894+
* never apply here because the path under test never reads the context; likewise [send] throws
895+
* rather than returning a stub response, as the synchronous path is never reached.
892896
*/
893897
private class SyncThrowingDispatchClient(
894898
private val failure: RuntimeException,

0 commit comments

Comments
 (0)