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

Proper way to dispose of Response [SPR-17268] #21801

Closed
spring-issuemaster opened this issue Sep 11, 2018 · 2 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Sep 11, 2018

David Terk opened SPR-17268 and commented

Recently there was a fix that addressed some potential memory leaks if there were errors in the response to the Webclient.  Fix is here

#21563

If using exchange() what is the proper way to dispose of a response?  Can leaks still occur on bodyToMono(String.class) ? Trying to understand if this fix was specific to cases where bytebuffers are used e.g. 

return join(response.body(BodyExtractors.toDataBuffers()))
 .map(dataBuffer -> {
 byte[] bytes = new byte[dataBuffer.readableByteCount()];
 dataBuffer.read(bytes);
 DataBufferUtils.release(dataBuffer);
 return bytes;
 }).then();

or everywhere.

The above snippet is used directly in WebClients default error handler - so would like to know if this is the only "safe" way to deal with disposing a response if a non 200 status code has taken place after using exchange.

 


Affects: 5.0.8

Reference URL: #21563

Issue Links:

  • #21563 DataBufferUtils#join could leak buffers in case of error from the source
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Sep 14, 2018

Arjen Poutsma commented

The proper way to deal with ClientResponse is listed in its javadoc:

When given access to a ClientResponse through the WebClient exchange() method, you must always use one of the body or toEntity methods to ensure resources are released and avoid potential issues with HTTP connection pooling. You can use bodyToMono(Void.class) if no response content is expected. However keep in mind that if the response does have content, the connection will be closed and will not be placed back in the pool.

So if you're using bodyToMono(String.class), it will not leak.

The fix fix was for a situation where a stream multiple data buffers where to be joined into one, but where the given stream contained an error signal. This can occur, for instance, when an underlying socket or file can no longer be read. Previously, this would result in memory leaks. Now, it no longer does. Joining data buffers is something that is done a lot in the decoders, so internally in the framework.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Sep 14, 2018

David Terk commented

Arjen Poutsma thanks just wanted to make sure that if there were any errors while the String was being composed from the response that it would not leak as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.