Skip to content
New issue

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

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

Already on GitHub? # to your account

LEAK: ByteBuf.release() was not called before it's garbage-collected #13

Open
wjzhuf opened this issue Jan 24, 2018 · 2 comments
Open

Comments

@wjzhuf
Copy link

wjzhuf commented Jan 24, 2018

When I use netty-http-client I found this error,when I modified class ResponseHandler ,the error disappears。I do not know if this is a problem, so I will ask。My English is poor。thankx

add :

io.netty.util.ReferenceCountUtil.release(content);

protected void internalReceive(HttpResponseStatus status, HttpHeaders headers, ByteBuf content) {
try {
if (status.code() > 399) {
onErrorResponse(status, headers, content.readCharSequence(content.readableBytes(), CharsetUtil.UTF_8).toString());
return;
}
MediaType mediaType = null;
if (headers.contains(HttpHeaderNames.CONTENT_TYPE)) {
try {
mediaType = MediaType.parse(headers.get(HttpHeaderNames.CONTENT_TYPE));
} catch (Exception ex) {
//do nothing
}
}
T o = mediaType == null ? marshallers.read(type, content) : marshallers.read(type, content, mediaType);
_doReceive(status, headers, type.cast(o));
} catch (Exception ex) {
content.resetReaderIndex();
try {
String s = Streams.readString(new ByteBufInputStream(content), "UTF-8");
onErrorResponse(HttpResponseStatus.REQUESTED_RANGE_NOT_SATISFIABLE, headers, s);
} catch (IOException ex1) {
ex.addSuppressed(ex1);
}
Exceptions.chuck(ex);
} finally {
//2018-01-24 修改测试
io.netty.util.ReferenceCountUtil.release(content);
latch.countDown();
}
}

@timboudreau
Copy link
Owner

Yes, I understand the problem, and if this fix works for you, great. Thank you for contributing the patch.

However, it is not possible to incorporate it into the code as-is, because it will break many clients which try to use the content after it has been released (that will throw an exception).

With some redesign of the API, it would be possible to change that. For now, the best approach is either to use a non-reference counted ByteBufAllocator, or for the code which receives the ByteBuf to dispose it once it knows it will not use it anymore.

@wjzhuf
Copy link
Author

wjzhuf commented Jan 24, 2018

@timboudreau
This increase is currently suitable for my application scenario, 1200 threads running for half an hour without leak problems。thank you for your reply。

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants