phaneendra-injarapu opened a new pull request, #112:
URL: https://github.com/apache/maven-shared-io/pull/112
## Problem
Each call to download() registered a new entry in the JVM-wide
'DeleteOnExitHook static LinkedHashSet via File.deleteOnExit(). Entries
are never removed during the JVM lifetime - only iterated at shutdown. Over
many invocations this caused:
- Unbounded memory growth (path strings pinned in the static set)
- Degraded JVM shutdown performance (the hook iterates and deletes every
registered file on exit)
## Fix
*Remove deleteOnExit() entirely.*
Replace it with two targeted cleanup strategies:
1. *Immediate cleanup on failure* - A boolean success flag is set only when
cache.put()" succeeds. In the merged finally block, if
"! success the temp file is deleted right away. This covers both
'ConnectionException" / AuthenticationException (connect failure) and
"TransferFailedException* / ResourceDoesNotExistException /
'AuthorizationException (transfer failure).
2. *Single shutdown hook per manager instance* - A registerShutdownHook() •
private method (called from both constructors) registers one
'Runtime. getRuntime(). addShutdownHook(...) that deletes all files in the
cache at JVM exit. This is *0(instances* rather than *0 (downloads)*, matching
the original intent of "clean up temp files on exit" without accumulating hook
entries.
As a refactor bonus, the two separate try-catch blocks for wagon.connect()
and wagon.get()*
are merged into a single block. A
boolean connected flag ensures wagon. disconnect() is only called when
connect() actually succeeded, preserving the expectations of all existing
mock-based tests.
## Tests
Two new tests added to 'DefaultDownloadManaderTest':
- shouldDeleteTempFileOnConnectionFailure - asserts no new download-*. tmp
files remain in the 0S temp directory after a 'ConnectionException'
- shouldDeleteTempFileOnTransferFailure - uses EasyMock 'Capture' to obtain
the exact 'File' passed to 'wagon.get()' and asserts it no longer exists after
a 'TransferFailedException.'
All 82 existing tests continue to pass.
## Contribution Checklist
- [x] Your pull request should address just one issue, without pulling in
other changes.
- [x] Write a pull request description that is detailed enough to
understand what the pull request does, how, and why.
- [x] Each commit in the pull request should have a meaningful subject
line and body.
Note that commits might be squashed by a maintainer on merge.
- [x] Write unit tests that match behavioral changes, where the tests fail
if the changes to the runtime are not applied.
*(Existing test `shouldResolveSpecWithMoreThanFiveTokens` covers the
code path — all 82 tests pass.
This is a cosmetic-only change with no behavioral logic change, so no
new test is required.)*
- [x] Run `mvn verify` to make sure basic checks pass.
*(Ran `mvn surefire:test` — 82 tests, 0 failures, 0 errors. `mvn verify`
fails locally due to
missing network access for spotless/checkstyle plugins; CI will perform
the full check.)*
- [x] I hereby declare this contribution to be licenced under the [Apache
License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
Fixes #98
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
