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

[Jason Mehrens]

Would it work to fix this by making DeleteOnExitHook::runHooks deal with dependencies?
1. Remove If deleted, or not directory which also takes care of not exists.
2. Sort remaining files by deepest child files/directories first.
3. Run delete again on the list.

Otherwise files need to be processed in reverse order before directories and directories need to be processed children first up to the root.

Jason

________________________________________
From: core-libs-dev <core-libs-dev-bounces at openjdk.java.net> on behalf of Ivan Gerasimov <ivan.gerasimov at oracle.com>
Sent: Wednesday, July 10, 2019 8:51 PM
To: Brian Burkhalter; core-libs-dev
Subject: Re: 8193072: File.delete() should remove its path from DeleteOnExitHook.files


On 7/10/19 5:17 PM, Brian Burkhalter wrote:
> I incorporated Peter’s version, adding the security check in cancelDeleteOnExit(), tweaking its verbiage along with that of deleteOnExit(), and modified the test DeleteOnExit to verify the new method. The updated version is here:
>
> http://cr.openjdk.java.net/~bpb/8193072/webrev.03/ <http://cr.openjdk.java.net/~bpb/8193072/webrev.03/>
There is possibility of a race here in a scenario like this:

File dir = new File("dir");
File file = new File("dir/file");

-- thread 1 --
dir.deleteOnExit();
file.deleteOnExit();
///
dir.cancelDeleteOnExit();
////  thread 2 intervenes
file.cancelDeleteOnExit();
-- end --

-- thread 2 --
dir.deleteOnExit();
file.deleteOnExit();
-- end --

The result will be that the order of the registered files will change,
and JVM will try to delete dir first (this will fail as it is not empty).

Of course it could be avoided, if cancellation were done in reverse
order, though it's not immediately obvious from the documentation.

With kind regards,
Ivan
> Thanks,
>
> Brian
>
>> On Jul 10, 2019, at 11:17 AM, Brian Burkhalter <brian.burkhalter at oracle.com> wrote:
>>
>> On Jul 10, 2019, at 5:36 AM, Peter Levart <peter.levart at gmail.com> wrote:
>>> There are various interleavings of threads that could cause the file to be left undeleted when VM exits.
>>>
>>> To cover concurrent scenarios such as above, the code could use reference counting. Like in the following patch:
>>>
>>> http://cr.openjdk.java.net/~plevart/jdk-dev/8193072_File.undoDeleteOnExit/webrev.01/ <http://cr.openjdk.java.net/~plevart/jdk-dev/8193072_File.undoDeleteOnExit/webrev.01/>
>> This looks good to me modulo adding this
>>         SecurityManager security = System.getSecurityManager();
>>         if (security != null) {
>>             security.checkDelete(path);
>>         }
>> to cancelDeleteOnExit() as suggested below.
>>
>>> On Jul 10, 2019, at 7:51 AM, Sean Mullan <sean.mullan at oracle.com> wrote:
>>>
>>> On 7/9/19 7:40 PM, Brian Burkhalter wrote:
>>>> I don’t know. On the one hand this does not take an action like reading, writing, or deleting, but on the other it could end up causing files to be left lying around after VM termination which were expected to be deleted. I suppose that could be considered to be some sort of security issue.
>>> Yes I think so.
>>>
>>> I would probably just use the same permission (FilePermission(file,"delete")). If you have been granted permission to delete a file, then you should have permission to cancel that deletion as well.
>> That’s a  good idea.

--
With kind regards,
Ivan Gerasimov