Prevent file handle leak during individual purge #707
Conversation
…ivative directory to avoid file handle leak
| try { | ||
| return Files.list(cachePath) | ||
| try (final var fileStream = Files.list(cachePath)) { | ||
| return fileStream | ||
| .filter(p -> p.getFileName().toString().startsWith(expectedNamePrefix)) |
There was a problem hiding this comment.
@bbpennel. Nice! so the try() will auto close the fileStream... But... is the filter needed? Given the way caches are structured all files inside cachePath will need to be returned. Maybe for sanity just check if the file is regular file?
.filter(Files::isRegularFile) or something like that? I mean, just to save a few milliseconds per cycle... specially on large images with tons of derivatives
There was a problem hiding this comment.
The filter isn't a change in this PR, i didn't really evaluate whether it was needed or not. I'm fine with removing it if there isn't any concern of the cache path containing files that don't begin with the md5.
There was a problem hiding this comment.
@bbpennel thanks, true!, for the sake of not adding any point of rupture and because I don't know what use case the original Developer might had had in place, let's keep it! Approving. Thanks!
|
Cantaloupe group met and like this fix. If you can respond to the filter question reaised above then we can look at merging. |
|
@bbpennel there is a test failing (looks related... but might be yet again a transient/timeout error on writing a file/reading it) |
#706
Use a try-with-resource block when getting the stream of files in derivative directory to avoid file handle leak