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

Add information about the need to release netty's buffers after message processing in WebSocket #112

Open
yeputons opened this issue Aug 31, 2015 · 4 comments

Comments

@yeputons
Copy link

Say, websocker example does not call frame.frame.release() after it finishes processing, which results in memory leak. You can read more about reference-counted objects in Netty here.

One possible way to check that you have no easy-detectable memory leaks is to run the application with -Dio.netty.leakDetectionLevel=paranoid, run garbage collector a lot with System.gc() and look for errors in logs.

@yeputons yeputons changed the title Add information about the need to release netty's buffers after message processing Add information about the need to release netty's buffers after message processing in WebSocket Aug 31, 2015
@simbo1905
Copy link

Can you point out the example code which has the problem and provide a gist of the fix?

@simbo1905
Copy link

So what I am reading is that the change would look like this in a typical socko app simbo1905/sprint-planning@6faec3f

Do we have to leave it down to the person writing the app to remember to call release? Would it not be better and safer for socko to always call release after it has run the application supplied code?

@yeputons
Copy link
Author

yeputons commented Sep 3, 2015

@simbo1905 Yep, that's what it looks like. However, some applications may want to pass buffers internally: say, to other actors. So if Socko suddenly start releasing buffer, it will be very surprising backward-incompatible change - apps will either crash or get wrong data because the buffer they try to access is already recycled.

@simbo1905
Copy link

@yeputons okay but the vanilla use case is to parse the buffer into some app immutable type then pass on the app type for processing freeing the buffer. handing the release for the vanilla use case is error prone as exceptions may be thrown. how about this version which has a releaseAfter(f) { doNormalWork }? if that looks usable then i would suggest you or i should fork the demo code here and create a PR to have that helper function merged back into this repo.

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

No branches or pull requests

2 participants