On 28/01/2013 19:02, Dan Xu wrote:
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
canonicalize_md.obj.
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
-out:C:/jprt/T/P1/183441.dan/s/build/windows-amd64/tmp/java/java.nio/nio/obj64/nio.dll
\
"-map:C:/jprt/T/P1/183441.dan/s/build/windows-amd64/tmp/java/java.nio/nio/obj64/nio.map"
\ -nologo -opt:REF -incremental:no -debug
-LIBPATH:C:/DXSDK/Lib/x64
@C:/jprt/T/P1/183441.dan/s/build/windows-amd64/tmp/java/java.nio/nio/obj64/nio.lcf
\ C:/jprt/T/P1/183441.dan/s/build/windows-amd64/lib/jvm.lib
ws2_32.lib
-libpath:C:/jprt/T/P1/183441.dan/s/build/windows-amd64/lib
java.lib
C:/jprt/T/P1/183441.dan/s/build/windows-amd64/tmp/java/java.nio/nio/obj64/../../../../sun/java.net/net/obj64/net.lib
C:/jprt/T/P1/183441.dan/s/build/windows-amd64/tmp/java/java.nio/nio/obj64/../../../java.lang/java/obj64/io_util.obj
C:/jprt/T/P1/183441.dan/s/build/windows-amd64/tmp/java/java.nio/nio/obj64/../../../java.lang/java/obj64/FileDescriptor_md.obj
advapi32.lib Creating library
C:/jprt/T/P1/183441.dan/s/build/windows-amd64/tmp/java/java.nio/nio/obj64/nio.lib
and object
C:/jprt/T/P1/183441.dan/s/build/windows-amd64/tmp/java/java.nio/nio/obj64/nio.exp
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.
I think the link failure you are seeing is because io_util.obj and
FileDescriptor_md.obj are also being linked into nio.dll, they shouldn't
be but perhaps someone added them a long time ago to workaround another
issue.
I checked the build of the JPLIS agent (and instrument.dll specifically)
and it just links canonicalize_md into that DLL. That is a bit messy,
and I remember some of the history as to why this was done this way, but
I don't think it should impact anything here.
:
- 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.
Okay, it would good to check if we have any tests because file
operations on "/" can have subtle difference to other directories, at
least on Solaris.
- 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?
It can be interrupted but the issue is that the file descriptor is
invalid at that point so we can't call close with it again.
- 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);
}
#endif
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.
I don't think we need this because the file descriptor will be closed at
exec time anyway (there is logic in Runtime.exec to iterate over the
file descriptors and close them). Also we don't do this in other areas
of the platform where we use open directly.
- 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?
I think it should be (size > current) ? (size - current) : 0.
Alternatively MAX(size-current, 0).
- 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.
Thanks, I suspect that some/most of these are not needed.
-Alan.
