Fix connection cache eviction - #55
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes eviction/cleanup behavior in Net::OpenSSH::ConnectionCache so that failed/undef entries are removed correctly and the cache-size eviction loop can run until the cache drops below the configured limit (making room for a new entry). It also adds a regression test to cover the previously broken cleanup paths.
Changes:
- Fix cleanup logic so failed cache entries are deleted and undef entries don’t trigger method calls.
- Fix eviction-loop stop condition so the loop can actually evict/weak-delete entries until the cache is below the threshold.
- Add a regression test for live/failed/undef entries and include it in
MANIFEST.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/Net/OpenSSH/ConnectionCache.pm | Rewrites cache cleanup and corrects the eviction loop stop condition. |
| t/connection-cache.t | Adds regression coverage for undef/failed/live cache entries and clean_cache behavior. |
| MANIFEST | Includes the new test file in the distribution manifest. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for (keys %cache) { | ||
| $ssh = $cache{$_}; | ||
| delete $cache{$_} unless $ssh and $ssh->error != OSSH_MASTER_FAILED; | ||
| } | ||
| for (keys %cache) { |
There was a problem hiding this comment.
Fixed in commit 195af61 by reindenting the cleanup block to match the surrounding 4-space style. Re-ran perl -Ilib -c lib/Net/OpenSSH/ConnectionCache.pm and perl -Ilib t/connection-cache.t.
| ok((keys %Net::OpenSSH::ConnectionCache::cache) <= $Net::OpenSSH::ConnectionCache::MAX_SIZE, | ||
| 'cache is reduced below the configured maximum'); |
There was a problem hiding this comment.
Fixed in commit 195af61 by aligning the assertion text with the <= MAX_SIZE condition. Re-ran perl -Ilib t/connection-cache.t.
Summary
Fixes two bugs in
Net::OpenSSH::ConnectionCache:Changes
MANIFEST.Fixes #35.
Fixes #36.
Testing
perl -Ilib -c lib/Net/OpenSSH/ConnectionCache.pmperl -Ilib t/connection-cache.t