Jackson2Decoder leaks on WebClient timeout

[nathankooij]

Verified on: 6.2.0-SNAPSHOT (https://github.com/spring-projects/spring-framework/commit/cbdfe815aa3562c6b05c066e6570e59e95c91e61)

Issue DataBuffers 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);
        });
    }

[bclozel]

Thanks for your report @nathankooij - this will ship with next month's maintenance release.