fix: Fix resource leak by wrapping GroovyClassLoader in try-with-resources#1497
fix: Fix resource leak by wrapping GroovyClassLoader in try-with-resources#1497ArpanC6 wants to merge 2 commits intojenkinsci:mainfrom
Conversation
|
"The linux-25 test failures appear to be pre-existing and unrelated to this change. The windows-21 build passes successfully with the same code. This PR only wraps GroovyClassLoader in try-with-resources and does not modify any script execution logic." |
ArpanC6
force-pushed
the
fix/resource-leak-clean
branch
from
fd291e0 to
6ba51fc
Compare
|
The test failures appear to be pre-existing and unrelated to this change. |
|
The CodeQL security scan failure is unrelated to this PR. The build fails in MailAccount.java at line 170 due to a pre-existing API incompatibility — org.acegisecurity.Authentication cannot be converted to org.springframework.security.core.Authentication. This file was not modified in this PR. The same compilation error would occur on any PR that triggers the CodeQL autobuild. |
| gloader.addURL(entry.getURL()); | ||
| } | ||
| gloader.addURL(entry.getURL()); | ||
| loader = gloader; |
There was a problem hiding this comment.
Won't this still keep it alive since it is being reassigned to loader?
|
Hi @slide I've addressed your feedback
Please let me know if any further changes are needed. Thank you.. |
ArpanC6
force-pushed
the
fix/resource-leak-clean
branch
from
14d7a15 to
1820f05
Compare
|
Hi @slide I've squashed the commits and the PR now contains only the single file change to ExtendedEmailPublisher.java. Please let me know if any further changes are needed. Thank you... |
ArpanC6
force-pushed
the
fix/resource-leak-clean
branch
from
41b0cba to
8b54785
Compare
|
Hi @slide I've fixed the resource leak by closing the GroovyClassLoader in a finally block within executeScript() after script execution completes rather than in expandClasspath(). This ensures the classloader stays alive during script execution but is properly closed afterward. All 365 tests are now passing. Please let me know if any further changes are needed. Thank you.. |
|
Hi @slide just following up on this PR. Please let me know if any further changes are needed. Happy to update. Thank you.. |
|
@slide just wanted to check in on this PR. I have addressed all the |
3 participants
Summary
Fixes a resource leak in GroovyClassLoader by wrapping it in try-with-resources.
Changes
Testing