8193072: File.delete() should remove its path from DeleteOnExitHook.files

[Ivan Gerasimov]

Hi Brian!

I believe this would introduce a behavior change in a scenario lile:
File f = ...;
f.deleteOnExit();
f.delete();
f.createNewFile();

I.e. when the with the same name is recreated.  Current behavior is to 
unlink such a file before exiting, no matter if it were explicitly 
deleted and then recreated or not.

With kind regards,
Ivan


On 7/8/19 1:11 PM, Brian Burkhalter wrote:
> https://bugs.openjdk.java.net/browse/JDK-8193072 <https://bugs.openjdk.java.net/browse/JDK-8193072>
>
> There does appear to be a memory leak of sorts if one does something like
>
> —
>
> File[] files;
> for (int i = 0; i < largeNumber; i++) {
>      files[i] = File.createTempFile(“blah”, null);
>      files[i].deleteOnExit();
> }
>
> // do something
>
> for (int i = 0; i < largeNumber; i++) {
>      files[i].delete();
> }
>
> // do something else before shutdown
>
> —
>
> The LinkedHashSet in DeleteOnExitHook will contain at least largeNumber Files until the VM shuts down even though the files were deleted.
>
> The potential change is included below. The additional call to DeleteOnExitHook.remove() in File.delete() does not appear to have a measurable performance impact, at least trivially and in isolation.
>
> Thanks,
>
> Brian
>
> --- a/src/java.base/share/classes/java/io/DeleteOnExitHook.java
> +++ b/src/java.base/share/classes/java/io/DeleteOnExitHook.java
> @@ -64,6 +64,15 @@
>           files.add(file);
>       }
>   
> +    static synchronized void remove(String file) {
> +        if(files == null) {
> +            // DeleteOnExitHook is running. Too late to remove a file
> +            throw new IllegalStateException("Shutdown in progress");
> +        }
> +
> +        files.remove(file);
> +    }
> +
>       static void runHooks() {
>           LinkedHashSet<String> theFiles;
>   
> --- a/src/java.base/share/classes/java/io/File.java
> +++ b/src/java.base/share/classes/java/io/File.java
> @@ -1050,6 +1050,7 @@
>           if (isInvalid()) {
>               return false;
>           }
> +        DeleteOnExitHook.remove(path);
>           return fs.delete(this);
>       }
>
>
-- 
With kind regards,
Ivan Gerasimov