Losing features of JVM_Open, e.g. CLOEXEC

[Alan Bateman]

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