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

Explicitly close input stream in CompoundJarURLStreamHandler #4063

Conversation

@camlow325
Copy link
Contributor

@camlow325 camlow325 commented Aug 11, 2016

This commit ensures that the stream which is opened for a URL within
CompoundJarURLStreamHandler is explicitly closed before it falls out
of scope. Previously, the stream would still eventually be closed but
only after Garbage Collection.

This commit ensures that the stream which is opened for a `URL` within
`CompoundJarURLStreamHandler` is explicitly closed before it falls out
of scope.  Previously, the stream would still eventually be closed but
only after Garbage Collection.
@kares kares added the JRuby 1.7.x label Aug 11, 2016
@kares kares added this to the JRuby 1.7.26 milestone Aug 11, 2016
@kares kares merged commit 11aad5b into jruby:jruby-1_7 Aug 11, 2016
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
@enebo
Copy link
Member

@enebo enebo commented Aug 11, 2016

@kares should this get merged to master?

@camlow325
Copy link
Contributor Author

@camlow325 camlow325 commented Aug 11, 2016

@enebo It didn't look to me like the CompoundJarURLStreamHandler class exists in the current code on master. Is that right? Also, I did some testing with fairly recent release off of master, 9.1.2.0, and wasn't able to reproduce the issue with an excessive number open file descriptors from the "stdlib" jar. Do you think there's anything worth merging to master from this, then?

@enebo
Copy link
Member

@enebo enebo commented Aug 11, 2016

@camlow325 you are correct! It is not there. I was just double checking since I saw a PR merged to 1.7. Better to ask than forget.

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

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.