Review Request: JDK-8001334 - Remove use of JVM_* functions from code

Dan Xu dan.xu at
Mon Jan 28 19:02:32 UTC 2013

Hi Alan,

On 01/25/2013 06:53 AM, Alan Bateman wrote:
> Dan,
> I've taken a pass over this and overall this is good clean-up and 
> something that was just not possible before jdk8 because of the need 
> to keep -XX:+UseVMInterruptibleIO working on Solaris.
> A few comments:
> - I don't understand why these changes require the make file changes 
> to link io_util_md and canonicalize_md into nio.dll.
These two .obj are needed during the link process in windows platform. 
Because getLastErrorString functions, used in io_util.c, are inside 
io_util_md.obj. And after adding io_util_md.obj, it also introduces 
another dependency on getPrefixed function which is implemented in 

Here is the first exception I got if I keep make files untouched.

    The compilation passed. But the link afterwards failed,
    c:/PROGRA~2/MICROS~2.0/VC/Bin/AMD64/link -dll
    \       -nologo -opt:REF -incremental:no -debug
    \        C:/jprt/T/P1/183441.dan/s/build/windows-amd64/lib/jvm.lib
    -libpath:C:/jprt/T/P1/183441.dan/s/build/windows-amd64/lib java.lib
    advapi32.lib    Creating library
    and object
    io_util.obj : error LNK2019: unresolved external symbol
    getLastErrorString referenced in function throwFileNotFoundException

I wonder whether I can avoid these link dependencies if I move those 
functions into other files. But I found it might cause other issues. 
Here is the comment for getPrefixed() function.

    The appropriate location of getPrefixed() should be io_util_md.c,
    but java.lang.instrument package has hardwired canonicalize_md.c
    into their dll, to avoid complicate solution such as including
    io_util_md.c into that package, as a workaround we put this method here.

> - In the definition of ISNANF on Solaris it uses isnand. I'll bet the 
> comment in globalDefinitions_xxx dates back to the original port and 
> might not be true anymore (I'm not suggesting you change it without 
> intensive testing, just a comment that I'll bet it is not an issue now).
> - "FD" is probably okay, I just wonder about how easy it might be to 
> get a conflict.
FD is defined to jlong on windows and to jint on other platforms. The 
way of using FD is the same as the way of defining IO_Read, IO_write, 
and other similar functions. I think the conflict is minimum. This makes 
the code easier to maintain and understand.
> - src/solaris/native/java/io/UnixFileSystem_md.c, did you test access 
> to "/", I just wonder if it would be better to keep the existing test.
The current change works the same as the old logic. The old one assign 
JVM_EEXIST to fd if it is root directory, and bypass JVM_EEXIST error.In 
the current logic, it also only handles the condition if it is not the 
root directory.
> - handleClose in src/solaris/native/java/io/io_util_md.c, close is not 
> restartable (I think we have this wrong in a few places).
I see one of errors that close() may encounter is EINTR in its man page, 
which means it can be interruptible, right?
> - handleOpen - looks like a bug where it should use "mode" instead of 
> the 0666.
You are right. I will fix it.
> - I don't think the O_CLOEXEC code is needed in handleOpen, was this 
> copied from HotSpot?
In the original hotspot code, it has one section to enable the 
close-on-exec flag for the new file descriptor as the following.

             #ifdef FD_CLOEXEC
                     int flags = ::fcntl(fd, F_GETFD);
                     if (flags != -1)
                         ::fcntl(fd, F_SETFD, flags | FD_CLOEXEC);

According to the comment, "All file descriptors that are opened in the 
JVM and not specifically destined for a subprocess should have the 
close-on-exec flag set.  If we don't set it, then careless 3rd party 
native code might fork and exec without closing all appropriate file 
descriptors (e.g. as we do in closeDescriptors in UNIXProcess.c), and 
this in turn might:
      - cause end-of-file to fail to be detected on some file 
descriptors, resulting in mysterious hangs, or
      - might cause an fopen in the subprocess to fail on a system 
suffering from bug 1085341."

And the recent open() function (since Linux 2.6.23) already has 
O_CLOSEXEC flag to enable this flag by default. And it is a preferred 
way according to the man page, " Specifying this  flag permits  a 
program  to  avoid  additional fcntl(2) F_SETFD operations to set the 
FD_CLOEXEC flag.  Addi-tionally, use of this flag is essential in some 
multithreaded programs since using a separate fcntl(2)  F_SETFD  
operation to set the FD_CLOEXEC flag does not suffice to avoid race 
conditions where one thread opens a file descriptor at the same time  
as  another  thread  does  a fork(2) plus execve(2).
Additionally, if O_CLOEXEC is not supported, F_DUPFD_CLOEXEC is 
preferred than FD_CLOEXEC, which is what hotspot is using right now.

> - handleAvailable - assume the file is changing, in which case this 
> could return a negative available. I see the existing code lseeks to 
> the end whereas you are using the size from the stat, I think that is 
> okay, just need to properly handle the case that the file is truncated 
> between calls.
I have the code to lseek to the end if size < 0. Should I change it to 
size < current?
> - getLastErrorString => I wonder if we should move to strerror_r (less 
> ambiguity as to whether it is thread-safe). Probably best to use 
> strncpy too.
I will change to use strerror_r.
> - src/solaris/native/java/io/io_util_md.h - minor nit, alignment of 
I will fix it.
> - src/windows/native/java/io/io_util_md.h - it's not obvious to me why 
> these need JNIEXPORT but this might be tied into my first question 
> about the make changes.
Actually, I don't know why the old code needs JNIEXPORT, neither. I only 
change it to use FD. And I did not touch other parts. And for those new 
method declarations I added to solaris/native/java/io/io_util_md.h, I 
did not add JNIEXPORT. I will check whether it can solve the link issue 
if I remove JNIEXPORT from the method signature.
> That's all I have. I assume you will run all tests on all platforms 
> before this is pushed.\
Sure, I will run it.


> -Alan
>> :
>> -------- Original Message --------
>> Subject:     Review Request: JDK-8001334 - Remove use of JVM_* 
>> functions from code
>> Date:     Mon, 21 Jan 2013 23:25:25 -0800
>> From:     Dan Xu <dan.xu at>
>> To:     nio-dev <nio-dev at>
>> Hi,
>> Interruptible I/O on Solaris has been highly problematicand undercut 
>> portability. And the long term plan is to remove it completely from 
>> JDK. In JDK6, the VM option, UseVMInterruptibleIO, was introduced as 
>> part of a phase-out plan to allow developers/customers to disable it. 
>> In JDK7, the default value of UseVMInterruptibleIO flag was changed 
>> to"false" to make Solaris blockingI/O operations uninterruptible by 
>> Java thread interruptsby default.
>> The last stepof this phase-out plan is to remove the feature 
>> completely. And it is good to do it now in JDK8. In this fix, I 
>> removed all related codes in IO area by replacing JVM_* functions 
>> with direct system calls. Please help review the proposed fix. 
>> Similar changes in networking area will be done via a different bug.
>> Bug:
>> webrev: 
>> <>
>> Best,
>> -Dan

More information about the core-libs-dev mailing list