Verified on: 6.2.0-SNAPSHOT (https://github.com/spring-projects/spring-framework/commit/cbdfe815aa3562c6b05c066e6570e59e95c91e61)
Issue
DataBuffer
s are not always properly released when theWebClient
is used to deserialize JSON responses with Jackson. This seems to happen in cases of cancellation/timeout. This happens spuriously in our production systems, but I have added a reproduction case below that demonstrates the issue.
This issue (and the reproduction case) is similar to https://github.com/spring-projects/spring-framework/issues/22384, however, since the StringDecoder
was being used (which explicitly releases data buffers), the issue in the Jackson2Decoder
went unnoticed.
Reproduction case:
diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientDataBufferAllocatingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientDataBufferAllocatingTests.java
index 8b67d28902..0ca075da1e 100644
--- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientDataBufferAllocatingTests.java
+++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientDataBufferAllocatingTests.java
@@ -211,6 +211,31 @@ class WebClientDataBufferAllocatingTests extends AbstractDataBufferAllocatingTes
.verify(Duration.ofSeconds(3));
}
+ @ParameterizedDataBufferAllocatingTest
+ void bodyToFluxWithCancellation(DataBufferFactory bufferFactory) {
+ setUp(bufferFactory);
+
+ record Dummy(String foo, String bar) {}
+
+ for (int i = 0; i < 32; i++) {
+ System.out.println("Iteration: " + i);
+ this.server.enqueue(new MockResponse()
+ .setResponseCode(200)
+ .setHeader("Content-Type", "application/json")
+ .setChunkedBody("[%s]".formatted(String.join(",", Collections.nCopies(10_000, "{\"foo\" : \"123\", \"bar\" : \"456\" }"))), 5));
+
+ long timeout = (long) (Math.random() * 5_000);
+ try {
+ this.webClient.get()
+ .retrieve()
+ .bodyToFlux(Dummy.class)
+ .blockLast(Duration.ofMillis(timeout));
+ } catch (IllegalStateException e) {
+ System.out.println("Time out: " + timeout);
+ }
+ }
+ }
+
private void testOnStatus(Throwable expected,
Function<ClientResponse, Mono<? extends Throwable>> exceptionFunction) {
Potential fix:
With the following diff
the tests pass again. I did not check if e.g. bodyToMono
and/or other decoders are affected.
diff --git a/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Decoder.java b/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Decoder.java
index f0b31f65a4..8608a087ba 100644
--- a/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Decoder.java
+++ b/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Decoder.java
@@ -32,6 +32,7 @@ import com.fasterxml.jackson.databind.ObjectReader;
import com.fasterxml.jackson.databind.exc.InvalidDefinitionException;
import com.fasterxml.jackson.databind.util.TokenBuffer;
import org.reactivestreams.Publisher;
+import org.springframework.core.io.buffer.PooledDataBuffer;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.util.context.ContextView;
@@ -162,7 +163,8 @@ public abstract class AbstractJackson2Decoder extends Jackson2CodecSupport imple
catch (IOException ex) {
sink.error(processException(ex));
}
- });
+ })
+ .doOnDiscard(PooledDataBuffer.class, DataBufferUtils::release);
});
}
Comment From: bclozel
Thanks for your report @nathankooij - this will ship with next month's maintenance release.