Problem
LoggableResponseBody.closeDelegateOnce() (the single-close guard added in #80) sets the
delegateClosed flag only after delegate.close() returns normally:
private fun closeDelegateOnce() {
if (!delegateClosed) {
delegate.close()
delegateClosed = true
}
}
If delegate.close() throws, delegateClosed is left false. Both close() and the over-cap
one-shot source (PrefixThenTailSource.close()) reach the delegate through this method from
independent entry points, so a failure on the first path leaves the guard unset and the second
path closes the delegate again.
On the over-cap path:
- The consumer closes the one-shot source →
closeDelegateOnce() → delegate.close() throws →
guard stays false.
- The consumer later closes the wrapper →
closeDelegateOnce() → delegate.close() runs a
second time.
ResponseBody.close() is documented as idempotent, so a conforming delegate tolerates the repeat
call. But the guard exists precisely to guarantee a single close for delegates whose handles are
not safe to close twice, and the error path silently breaks that guarantee.
Inconsistency
The drain-path error handler in drainAndCache() does the opposite: when closing the source after
a failed read throws, it still marks the delegate closed so a later close() is a no-op.
closeDelegateOnce() should match that behavior.
Suggested fix
Set the guard whether or not delegate.close() throws:
private fun closeDelegateOnce() {
if (delegateClosed) return
try {
delegate.close()
} finally {
delegateClosed = true
}
}
This keeps "closed at most once" true even when the close itself fails, and lets the exception
propagate as before.
Severity
Low — it only triggers when delegate.close() throws, and the public contract already asks
delegates to be idempotent. Worth fixing for consistency with the drain path and to keep the
single-close guarantee honest.
Problem
LoggableResponseBody.closeDelegateOnce()(the single-close guard added in #80) sets thedelegateClosedflag only afterdelegate.close()returns normally:If
delegate.close()throws,delegateClosedis leftfalse. Bothclose()and the over-capone-shot source (
PrefixThenTailSource.close()) reach the delegate through this method fromindependent entry points, so a failure on the first path leaves the guard unset and the second
path closes the delegate again.
On the over-cap path:
closeDelegateOnce()→delegate.close()throws →guard stays
false.closeDelegateOnce()→delegate.close()runs asecond time.
ResponseBody.close()is documented as idempotent, so a conforming delegate tolerates the repeatcall. But the guard exists precisely to guarantee a single close for delegates whose handles are
not safe to close twice, and the error path silently breaks that guarantee.
Inconsistency
The drain-path error handler in
drainAndCache()does the opposite: when closing the source aftera failed read throws, it still marks the delegate closed so a later
close()is a no-op.closeDelegateOnce()should match that behavior.Suggested fix
Set the guard whether or not
delegate.close()throws:This keeps "closed at most once" true even when the close itself fails, and lets the exception
propagate as before.
Severity
Low — it only triggers when
delegate.close()throws, and the public contract already asksdelegates to be idempotent. Worth fixing for consistency with the drain path and to keep the
single-close guarantee honest.