Release queued body token buffers on multipart cancel - #37115
Conversation
When a multipart subscriber cancels while MultipartParser has already emitted body tokens beyond the downstream demand, those tokens are held in the Flux.create sink queue (and in downstream operator queues such as windowUntil). On cancellation, Reactor discards the queued tokens, but BodyToken is not a DataBuffer, so the buffers inside the discarded tokens are never released and Netty reports "LEAK: ByteBuf.release() was not called before it's garbage-collected". Register a doOnDiscard hook for BodyToken in MultipartParser.parse() so that a discarded body token releases its buffer, both in the sink queue and in any downstream operator queue that supports discarding. Closes spring-projectsgh-37115 Signed-off-by: Hyunsik Kang <cj848@hanmail.net>
c97b6a2 to
93beb6b
Compare
BodyState.flush() emits every queued buffer and only clears the queue
afterwards, so a cancellation arriving while it emits makes dispose()
release buffers whose ownership has already been transferred to the sink.
Such a buffer is then released twice: once by the parser, and once by the
downstream consumer or the discard hook. With Netty, body buffers are
slices of the inbound buffer, so the second release frees the inbound
buffer prematurely, which surfaces as
IllegalReferenceCountException: refCnt: 0, decrement: 1
io.netty.handler.codec.http.DefaultHttpContent.release
reactor.netty.channel.FluxReceive.drainReceiver
when reactor-netty releases its own share right after onNext.
Remove each buffer from the queue before emitting it, mirroring what
enqueue() already does, so that dispose() only ever releases buffers the
parser still owns.
|
I pushed a second commit to this PR, because the discard hook added here exposes a related ownership defect in the same class.
private void flush() {
for (Iterator<DataBuffer> iterator = this.queue.iterator(); iterator.hasNext(); ) {
DataBuffer buffer = iterator.next();
boolean last = !iterator.hasNext();
MultipartParser.this.emitBody(buffer, last); // ownership handed to the sink
}
this.queue.clear(); // removed from the queue only now
}
@Override
public void dispose() {
this.queue.forEach(DataBufferUtils::release);
this.queue.clear();
}If a cancellation arrives while With Netty this is not just a log line: body buffers are slices of the inbound buffer, so the extra release frees the inbound buffer prematurely, and reactor-netty then fails when releasing its own share right after We observe this in production on a WebFlux service (1–5 occurrences per day under The fix removes each buffer from the queue before emitting it, so private void flush() {
DataBuffer buffer;
while ((buffer = this.queue.poll()) != null) {
MultipartParser.this.emitBody(buffer, this.queue.isEmpty());
}
}The added test Happy to split this into a separate PR if you prefer to keep the two changes independent. |
MultipartParseremits multiple tokens per inputDataBuffer(a headers token followed by body tokens), regardless of downstream demand. Tokens emitted beyond the demand are buffered in theFlux.createsink queue, and downstream operators used byDefaultPartHttpMessageReader/PartEventHttpMessageReader(windowUntil,concatMap) keep their own token queues as well.When the subscriber cancels while such tokens are queued — for example when the client aborts the connection mid-upload — Reactor discards the queued tokens.
BodyTokenis not aDataBuffer, so no existingdoOnDiscard(DataBuffer.class, ...)hook can reach the buffer it wraps, and the buffer is dropped without being released. With Netty this surfaces in production as:with leak-detector access records ending in
MultipartParser$BodyState.onNext/DataBufferUtils$AbstractNestedMatcher.matchand no matching release. This is related to, but distinct from, gh-36262: that fix coveredPartGenerator, while this leak happens one stage earlier, in the parser token stream itself.This PR registers a
doOnDiscard(BodyToken.class, ...)hook inMultipartParser.parse(). Since the discard handler is propagated upstream through the subscriber context, a single hook at this convergence point covers the sink queue as well as any downstream operator queue that supports discarding, for bothPartandPartEventreading.The new
MultipartParserTests.cancelWithQueuedBodyTokensReleasesBufferstest reproduces the leak deterministically (no race): a whole multipart message in a single input buffer with a downstream demand of 1 leaves the body token in the sink queue at cancellation time. With the fix reverted, the test fails with1 buffer leaks detected; with the fix applied, it passes.:spring-web:test --tests "org.springframework.http.codec.multipart.*",:spring-web:checkstyleMainand:spring-web:checkstyleTestpass locally.