Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jackson2Decoder leaks on WebClient timeout #33731

Closed
nathankooij opened this issue Oct 17, 2024 · 1 comment
Closed

Jackson2Decoder leaks on WebClient timeout #33731

nathankooij opened this issue Oct 17, 2024 · 1 comment
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Milestone

Comments

@nathankooij
Copy link

Verified on: 6.2.0-SNAPSHOT (cbdfe81)

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 #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);
 		});
 	}
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 17, 2024
@bclozel bclozel added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Oct 17, 2024
@bclozel bclozel self-assigned this Oct 17, 2024
@bclozel bclozel added type: bug A general bug in: core Issues in core modules (aop, beans, core, context, expression) and removed status: waiting-for-triage An issue we've not yet triaged or decided on in: web Issues in web modules (web, webmvc, webflux, websocket) labels Oct 18, 2024
@bclozel bclozel added this to the 6.1.15 milestone Oct 18, 2024
@bclozel
Copy link
Member

bclozel commented Oct 18, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants