-
Notifications
You must be signed in to change notification settings - Fork 32
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
Fix ByteBuf Leak! #77
base: master
Are you sure you want to change the base?
Conversation
Fix LEAK: ByteBuf.release() was not called before it's garbage-collected.
@@ -76,8 +76,12 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception | |||
reading = true; | |||
if (msg instanceof BinaryWebSocketFrame) { | |||
BinaryWebSocketFrame frame = (BinaryWebSocketFrame) msg; | |||
String json = frame.content().toString(StandardCharsets.UTF_8); | |||
messageHandler.handle(json); | |||
try { //TODO: @wjw_note: release BinaryWebSocketFrame Prevent resource leakage! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of this line ? can we remove it ? others LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use try finally
promise release BinaryWebSocketFrame Prevent resource leakage!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, I meant the comment: //TODO: @wjw_note: release BinaryWebSocketFrame Prevent resource leakage!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the note I added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry again that I did not make myself clear, and to be precise, it is TODO: @wjw_note:
, I don't want to be captious, but TODO
means something not ready, and the @wjw_note
looks like not this component related.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, the TODO & wjw__note
in this comment is misleading
Fix LEAK: ByteBuf.release() was not called before it's garbage-collected.