On 29/10/2014 20:12, Martin Buchholz wrote:
Hi guys,
In your change
8057777: Cleanup of old and unused VM interfaces
you have made changes like this:
- zfd = JVM_Open(path, flag, 0);
+ zfd = open(path, flag, 0);
throwing away use of old shared infrastructure and replacing with
"naked" calls to the OS. Although understandable, this abandons
benefits of using shared infrastructure. Here I'm thinking of the
close-on-exec flag. I just added use of O_CLOEXEC to linux hotspot,
but e.g. zip file opening no longer uses JVM_Open, which is a code
hygiene regression.
What we want is to have almost all file descriptors have the
close-on-exec flag automatically set at fd creation time, using
O_CLOEXEC where available, and FD_CLOEXEC where not. How do we get there?
I think Frederic's clean-up was generally good, it gets rid of a lot of
crud from early JDK versions. In early versions of the JDK then it was
important for the library code to go through the VM because the JDK
could be run on green threads so every potentially-blocking syscall had
to wrapped in the VM. There was also an attempt at a porting interface
at the time but this bit rotted a long time ago (much of it was orphaned
when we moved from the the classic VM to HotSpot). The remaining bits of
that porting interface were finally removed in JDK 7 but that still left
the JVM interface with a lot of functions, networking and some file I/O,
that aren't interesting for the JVM interface. So I think it's good to
have these removed from the JVM interface. Also good to see the no-op
functions to support java.lang.Compiler and Runtime.traceXXX go away too.
The change in behavior from JVM_Open ito open was missed but there is a
long way to go in JDK 9 so lots of time to fix the issue (if there is an
issue). As was noted in one of the replies, the only usages of JVM_Open
were in the zip code, it hasn't been used consistently used in the
libraries for at least 10+ years. When doing significant changes then
thorough code reviews are important but good tests are equally, and
sometimes more, important. I understand that having a test for the
O_CLOEXEC might be too hard so I'm interested to know how you ran into
it. Maybe it was inspection? Maybe an app that opens lots of zip files
and executes fork/exec itself? For ProcessBuilder and Runtime.exec
usages then the I thought the childproc code handled this by looping
over the open file descriptors and closing them. So even if O_CLOEXEC or
fcntl(FD_CLOEXEC) isn't used then it should be closed in the child.
As to whether we need a JVM_Open replacement in libjava then maybe we do
but I think it needs a discussion as to whether the JDK really needs a
porting interface or just useful infrastructure. Also I think going
forward then I assume that the amount of native code will reduce,
particularly when FFI comes along and we start to get rid of some of the
native code (at least I hope that happens).
In the mean-time then I think I'd like to understand more as to whether
the O_CLOEXEC is an issue or not. Aside from the zip code then I don't
think we've been using it consistently so I'm wondering if the issue is
some native code calling fork+exec or something else?
-Alan
