-
Notifications
You must be signed in to change notification settings - Fork 630
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
SpringBoot WebClient memory leak when the response is not consumed in the filter #1603
Comments
@gdinant In that memory leak stack I do not see any |
Oops, I surely got confused between reactor.netty and io.netty while googling for this specific exception 😢 which (always) led me here. I assume I can just reopen this case in netty's github. Sorry @violetagg! |
@gdinant I'm not saying that the issue is/is not in Rector Netty, just I do not see any |
@gdinant One additional question: You enabled |
Yes it is the only stack I can get (so far). Will post more if possible (and necessary).
Yes for the paranoid option. log level I set I've just tried with your configuration instead, but I don't get anything more. |
Another piece of information (might help too). Regarding the filters applied: private WebClient.Builder componentWebClientBuilder() {
return webClientBuilder.clone()
.filter(renewTokenFilter())
.filter(tokenFilter())
.filter(new CorrelationIdHeaderFilter(configuration.getCorrelationIdSupplier()).filter())
.filter(new PeerIdHeaderFilter(configuration.getPeerIdSupplier()).filter())
.filter(new LoggingFilter().filter())
.clientConnector(clientHttpConnector());
} They're all pretty dummy apart from token related filters which are making a downstream call in case of a 401 (token expiry). Here's an example of the implementation: public ExchangeFilterFunction tokenFilter() {
return ExchangeFilterFunction.ofRequestProcessor(req -> {
String token = tokenHolder.token();
return Mono.just(ClientRequest.from(req).header(ITokenHandler.TOKEN_HTTP_HEADER, token).build());
});
}
public ExchangeFilterFunction renewTokenFilter() {
return (req, next) -> next.exchange(req).flatMap((Function<ClientResponse, Mono<ClientResponse>>) res -> {
if (res.statusCode().value() == HttpStatus.UNAUTHORIZED.value()) {
tokenHolder.renewToken();
return next.exchange(req);
} else {
return Mono.just(res);
}
});
} I thought that maybe having an exception on the line |
@gdinant So in case of Is it possible instead of |
@violetagg you're right, and I'm aware of this pitfall regarding .exchange() on WebClient. However in this filter @FunctionalInterface
public interface ExchangeFunction {
/**
* Exchange the given request for a {@link ClientResponse} promise.
* @param request the request to exchange
* @return the delayed response
*/
Mono<ClientResponse> exchange(ClientRequest request);
/**
* Filter the exchange function with the given {@code ExchangeFilterFunction},
* resulting in a filtered {@code ExchangeFunction}.
* @param filter the filter to apply to this exchange
* @return the filtered exchange
* @see ExchangeFilterFunction#apply(ExchangeFunction)
*/
default ExchangeFunction filter(ExchangeFilterFunction filter) {
return filter.apply(this);
}
} Perhaps I still need to consume somehow the body indeed :). |
@rstoyanchev Can you take a look? |
WebClient request building -> [filter1 -> filter2 -> ... -> ExchangeFunction/ClientHttpConnector] -> WebClient response handling The Handle exceptions very carefully at that level and consume the response or call |
I eventually managed to reproduce the LEAK. Just needed to run the test for a longer time with more hits. The leak seems to disappear by adding the following line "consuming" the response body: public ExchangeFilterFunction renewTokenFilter() {
return (req, next) -> next.exchange(req).flatMap((Function<ClientResponse, Mono<ClientResponse>>) res -> {
if (res.statusCode().value() == HttpStatus.UNAUTHORIZED.value()) {
res.releaseBody().subscribe(); // LINE ADDED
tokenHolder.renewToken();
return next.exchange(req);
} else {
return Mono.just(res);
}
});
} However calling "blocking" functions inside of non-blocking code isn't recommended. If I were to replace @rstoyanchev would you see another workaround for doing so? or is it the right approach? -- EDIT I rewrited the function as follow which seems to be slightly better and get rid of the LEAK as well : public ExchangeFilterFunction renewTokenFilter() {
return (req, next) -> next.exchange(req).flatMap((Function<ClientResponse, Mono<ClientResponse>>) res -> {
if (res.statusCode().value() == HttpStatus.UNAUTHORIZED.value()) {
return res.releaseBody().thenReturn(tokenHolder.renewToken()).flatMap(t -> next.exchange(req));
} else {
return Mono.just(res);
}
});
} Tell me if you see anything wrong :) |
That looks good @gdinant |
Perfect then, closing the issue ;-). Thanks for your help! |
Hello everyone,
I'm running into a LEAK with WebClient. However I'm not able to reproduce the LEAK at all. The ERROR log is happening now and then in all live environments (roughly every 2-3 days).
I have read most of the other topics that got raised on the subject, but nothing seems to ressemble to my use case.
I'd be really grateful if anyone could help me out on this.
Expected Behavior
No LEAK
Actual Behavior
Standard stacktrace
In "advanced" logging mode:
Client configuration
There is only one client in this component configured as such:
ClientContext snippet:
And used as follow:
CreditClient snippet:
Side notes
I thought that it might be induced by the Timelimiter, but I couldn't manage to reproduce the leak either. The connection seems to close itself correctly too.
CreditClientTest snippet:
Reactor-netty: 1.0.5 (spring-boot-starter-webflux)
netty
, ...):Springboot: 2.4.4
Spring: 5.3.5
Resilence4j: 1.7.0
java -version
):openjdk version "11.0.9.1" 2020-11-04
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.9.1+1)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.9.1+1, mixed mode)
uname -a
):Linux cumulusvoucher-34-d4gwj 3.10.0-1062.12.1.el7.x86_64 #1 SMP Thu Dec 12 06:44:49 EST 2019 x86_64 x86_64 x86_64 GNU/Linux
The text was updated successfully, but these errors were encountered: