Hi,
this PR fixes #22108 by calling deleteOnExit on the temporary file and additionally prevents creating the file all together if we don't exceed the thresholds for which it was introduced.
Since deleteOnExit is only executed on "normal" VM termination, there is still a slight chance that temporary files will not be cleaned up when the VM dies for unknown reasons. But with the additional lazy init they probably won't be created in the first place for most apps.
Let me know what you think.
Cheers, Christoph
Comment From: wilkinsona
Thanks for the PR, @dreis2211. Ideally, I'd like a solution that can be tested but that's not easy to do with deleteOnExit(). I made an attempt that makes EntryWriter Closeable but the knock-on effect of that was broader than I'd like. Having spent some time in the code trying to find a nice solution, I've found myself wondering why we're using a file at all. It's inconsistent with how we handle calculating the hash for unpack comments which is currently done entirely in memory. I'm going to put this one on hold for now while we take a step back and consider things a bit more broadly.
Comment From: dreis2211
@wilkinsona I actually share the testability concern, I had this too while developing. My concern on the other hand is that - depending on how long this will be on-hold - we won't solve the bug for users in the upcoming 2.3.2 release. What about merging this for now to improve things for users, while opening a follow-up issue that tries to improve things on a broader level?
Comment From: wilkinsona
It would certainly be nice to get a fix in some form or other into 2.3.2. I've labelled #22108 for team attention so we'll hopefully discuss it later today. Let's see what comes out of that and then make a decision on the best way to proceed.
Comment From: dreis2211
Sounds reasonable. Thanks
Comment From: philwebb
We discussed this today and decided that we should merge this in 2.3.x and also bump up the threshold to reduce the number of files written. We'll perhaps try to look at refactoring that EntryWriter code sometime in the future to make them close aware.
Comment From: wilkinsona
Thanks again, @dreis2211!