-
Notifications
You must be signed in to change notification settings - Fork 903
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
[BugFix] Fix to prevent resource leaks in 3 classes #4449
Conversation
@@ -180,23 +180,24 @@ private synchronized BufferedLogChannel allocateNewLog(File dirForNextEntryLog, | |||
} while (testLogFile == null); | |||
|
|||
File newLogFile = new File(dirForNextEntryLog, logFileName); | |||
FileChannel channel = new RandomAccessFile(newLogFile, "rw").getChannel(); | |||
try (FileChannel channel = new RandomAccessFile(newLogFile, "rw").getChannel()) { |
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.
is it a good idea to close the channel here? it is used by the logChannel that is being returned
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.
Reverted changes for this file
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.
It seems we have been leaked some resources for long, really? Need more eyes. @nicoloboschi @hangc0276 @nodece
Looks like it. We should take care of it. |
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.
LGTM, but I think we need to change the commit and PR title. It's a fix of leak
Thanks for your contribution! |
Implemented try-with-resources for instances of Autocloseable