Skip to content

Commit 61e2df7

Browse files
committed
fix: keep single-close guard honest when delegate close() throws
LoggableResponseBody.closeDelegateOnce() set the delegateClosed flag only after delegate.close() returned normally. When that close throws, the flag was left false, so a second close() reaching the delegate through the other entry point (the over-cap one-shot PrefixThenTailSource.close() vs the wrapper's own close()) would close the delegate a second time. The guard exists precisely to guarantee a single close for delegates whose handles are not safe to close twice, and the error path silently broke that guarantee. Flip the guard in a finally so the delegate is marked closed whether or not its close() succeeds, while still letting the exception propagate. This also matches the drain-path error handler, which already marks the delegate closed after a failed source close so a later close() is a no-op. Closes #115
1 parent 80dfbf1 commit 61e2df7

2 files changed

Lines changed: 43 additions & 1 deletion

File tree

‎sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/LoggableResponseBody.kt‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,8 +210,14 @@ public class LoggableResponseBody
210210
*/
211211
@Throws(IOException::class)
212212
private fun closeDelegateOnce() {
213-
if (!delegateClosed) {
213+
if (delegateClosed) return
214+
// Flip the guard whether or not close() succeeds: a delegate whose handle is not safe
215+
// to close twice must still see exactly one close even when that close throws. Marking
216+
// it closed in a finally also matches the drain-path error handler, which marks the
217+
// delegate closed after a failed source close so a later close() is a no-op.
218+
try {
214219
delegate.close()
220+
} finally {
215221
delegateClosed = true
216222
}
217223
}

‎sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/response/LoggableResponseBodyTest.kt‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,42 @@ class LoggableResponseBodyTest {
474474
)
475475
}
476476

477+
@Test
478+
fun `delegate whose close throws is invoked only once across two close calls`() {
479+
// Over-cap path: the one-shot source close and the wrapper close both funnel through the
480+
// single-close guard. If the first close() throws, the guard must still flip so the second
481+
// close() is a no-op — a delegate whose handle is not safe to close twice must see exactly
482+
// one close even when that close fails.
483+
val delegateCloseCount = AtomicInteger(0)
484+
val payload = "abcdefghijklmnopqrstuvwxyz" // 26 bytes > cap
485+
val delegate =
486+
object : ResponseBody() {
487+
override fun mediaType(): MediaType? = MediaType.parse("text/plain")
488+
489+
override fun contentLength(): Long = payload.toByteArray(Charsets.UTF_8).size.toLong()
490+
491+
override fun source(): BufferedSource = Io.provider.buffer().also { it.writeUtf8(payload) }
492+
493+
override fun close() {
494+
delegateCloseCount.incrementAndGet()
495+
throw IOException("delegate close failed")
496+
}
497+
}
498+
val wrapper = LoggableResponseBody.bounded(delegate, Io.provider, 5L)
499+
500+
val tail = wrapper.source()
501+
// First close reaches the delegate and throws; the guard must still flip.
502+
assertFailsWith<IOException> { tail.close() }
503+
// Second close must be a no-op — the delegate is not closed again.
504+
wrapper.close()
505+
506+
assertEquals(
507+
1,
508+
delegateCloseCount.get(),
509+
"a delegate whose close() throws must still be closed exactly once across two close() calls",
510+
)
511+
}
512+
477513
@Test
478514
fun `over-cap close then source close closes the underlying source exactly once`() {
479515
// The reverse order: wrapper.close() first, then closing the already-handed-out one-shot

0 commit comments

Comments
 (0)