Seeking inputs on 8224794: ZipFile/JarFile should open zip/JAR file with FILE_SHARE_DELETE sharing mode on Windows

[Jaikiran Pai]

I have been experimenting with 
https://bugs.openjdk.java.net/browse/JDK-8224794 and have reached a 
stage with my changes where I would like some inputs before I move ahead 
or raise a PR for review.

As noted in that JBS issue, the idea is to open the underlying file 
using FILE_SHARE_DELETE when using java.uitl.zip.ZipFile and 
java.util.jar.JarFile. From what I can infer from that JBS issue, the 
motivation behind this seems to be to allow deleting that underlying 
file when that file is currently opened and in use by the 
ZipFile/JarFile instance. The linked github issues seem to suggest the 
same. IMO, it's a reasonable expectation, given that such a deletion 
works on *nix platform. It's on Windows where such a deletion fails with 
"The process cannot access the file because it is being used by another 
process" (the other process in many cases will be the current JVM 
instance itself). So trying to get this to work on Windows by setting 
the FILE_SHARE_DELETE share access mode seems reasonable.

Coming to the implementation part, I've an initial draft version here 
https://github.com/openjdk/jdk/pull/6477. It sets the FILE_SHARE_DELETE 
share access mode on the file when constructed by ZipFile/JarFile. It 
"works", in the sense that if you issue a delete against this underlying 
file (path) after its been opened by ZipFile/JarFile instance, you no 
longer see the error mentioned above and the delete completes without 
any exception. However, my experiments with this change have raised a 
bunch of questions related to this:

The crucial part of the "DeleteFile" API in Windows is that, as stated 
in its documentation[1]:

"The DeleteFile function marks a file for deletion on close. Therefore, 
the file deletion does not occur until the last handle to the file is 
closed. Subsequent calls to CreateFile to open the file fail with 
ERROR_ACCESS_DENIED."

So imagine the following pseudo-code:

String path = "C:/foo.jar"
1. JarFile jar = new JarFile(path); // create a jar instance
2. var deleted = new File(path).delete(); // delete the underlying file
3. var exists = new File(path).exists(); // check file existence
4. FileOutputStream fos = new FileOutputStream(new File(path)); // open 
a FileOutputStream for /tmp/foo.jar
5. java.nio.file.Files.deleteIfExists(Path.of(path));

When this is run (on Windows) *without* the change that introduces 
FILE_SHARE_DELETE, I notice that line#2 returns "false" for 
File.delete() (since the file is in use by the JarFile instance) and 
then line#3 returns "true" since it still exists and line#4 succeeds and 
creates a new FileOutputStream and line#5 fails with an exception 
stating "java.nio.file.FileSystemException: foo.jar: The process cannot 
access the file because it is being used by another process
"

Now when the same code is run (on Windows) *with* the proposed 
FILE_SHARE_DELETE changes, I notice that line#2 returns "true" (i.e. the 
file is considered deleted) and line#3 returns "true" (i.e. it's 
considered the file still exists). So essentially the file is only 
marked for deletion and hasn't been deleted at the OS level, since the 
JarFile instance still has an handle open. Then line#4 unlike 
previously, now throws an exception and fails to create the 
FileOutputStream instance. The failure says 
"java.nio.file.AccessDeniedException: foo.jar". This exception matches 
the documentation from that Windows DeleteFile API. line#5 too fails 
with this same AccessDeniedException.

So what this means is, although we have managed to allow the deletion to 
happen (i.e. the first call to delete does mark it for deletion at OS 
level), IMO, it still doesn't improve much and raises other difference 
in behaviour of APIs as seen above, unless the underlying file handle 
held by ZipFile/JarFile is closed. So that brings me to the next related 
issue.

For this change to be really useful, the file handle (which is an 
instance of RandomAccessFile) held by ZipFile/JarFile needs to be 
closed. If a user application explicitly creates an instance of 
ZipFile/JarFile, it is expected that they close it themselves. However, 
the most prominent use of the JarFile creation is within the JDK code 
where it uses the sun.net.www.protocol.jar.JarURLConnection to construct 
the JarFile instances, typically from the URLClassLoader. By default 
these JarFile instances that are created by the 
sun.net.www.protocol.jar.JarURLConnection are cached and the close() on 
those instances is called only when the URLClassLoader is close()ed. 
That in itself doesn't guarantee that the underlying file descriptor 
corresponding to the JarFile instance gets closed, because there's one 
more level of reference counting "cache" in use in 
java.util.zip.ZipFile$Source class where the same underlying file 
descriptor (corresponding to the jar file) gets used by multiple JarFile 
instances (irrespective of who created those). So in short, the 
underlying file descriptor of a JarFile only gets closed when every 
single instance of a JarFile pointing to the same jar file has been 
close()ed within the JVM. Which effectively means that introducing the 
FILE_SHARE_DELETE semantics/changes for Windows will only allow the 
first delete() attempt to succeed (since that one marks it for 
deletion), but any subsequent operations like the one noted in the 
examples, that rely on that file being deleted will continue to fail 
with Access Denied errors. Given all this caching of JarFile and the 
reference count cache for the underlying jar file, is this change of 
FILE_SHARE_DELETE adding much value and should I pursue it further?

[1] 
https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-deletefilew

-Jaikiran