Would there be interest in moving this forward? Enabling that
FILE_SHARE_DELETE option has opened up some questions that I noted in my
previous mail below. So in order to move forward I would need some
inputs. If we don't want to do this at this time, I'll close the draft
PR that's currently open https://github.com/openjdk/jdk/pull/6477.
-Jaikiran
On 19/11/21 5:10 pm, Jaikiran Pai wrote:
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
