RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
Staffan Larsen
staffan.larsen at oracle.com
Wed Jan 15 11:02:26 PST 2014
Yes, that looks like a good solution.
/Staffan
On 15 jan 2014, at 17:34, Volker Simonis <volker.simonis at gmail.com> wrote:
> Hi Staffan,
>
> thanks for the review. Please find my comments inline:
>
> On Wed, Jan 15, 2014 at 9:57 AM, Staffan Larsen <staffan.larsen at oracle.com> wrote:
> Volker,
>
> I’ve look at the following files:
>
> src/share/native/sun/management/DiagnosticCommandImpl.c:
> nit: “legel” -> “legal” (two times)
> In Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo() if you allow dcmd_info_array to become NULL, then jmm_interface->GetDiagnosticCommandInfo() will throw an NPE and you need to check that.
>
> Good catch. I actually had problems with malloc returning NULL in 'getDiagnosticCommandArgumentInfoArray()' and then changed all other potentially dangerous locations which used the same pattern.
>
> However I think if the 'dcmd_info_array' has zero length it would be perfectly fine to return a zero length array. So what about the following solution:
>
> dcmdInfoCls = (*env)->FindClass(env,
> "sun/management/DiagnosticCommandInfo");
> num_commands = (*env)->GetArrayLength(env, commands);
> if (num_commands = 0) {
> result = (*env)->NewObjectArray(env, 0, dcmdInfoCls, NULL);
> if (result == NULL) {
> JNU_ThrowOutOfMemoryError(env, 0);
> }
> else {
> return result;
> }
> }
> dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo));
> if (dcmd_info_array == NULL) {
> JNU_ThrowOutOfMemoryError(env, NULL);
> }
> jmm_interface->GetDiagnosticCommandInfo(env, commands, dcmd_info_array);
> result = (*env)->NewObjectArray(env, num_commands, dcmdInfoCls, NULL);
>
> That seems easier and saves me from handling the exception.
>
> What do you think?
>
> src/solaris/native/sun/management/OperatingSystemImpl.c
> No comments.
>
> src/share/transport/socket/socketTransport.c
> No comments.
>
> src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider
> No comments.
>
>
> Thanks,
> /Staffan
>
>
>
> On 14 jan 2014, at 09:40, Volker Simonis <volker.simonis at gmail.com> wrote:
>
>> Hi,
>>
>> could you please review the following changes for the ppc-aix-port stage/stage-9 repositories (the changes are planned for integration into ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage):
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/8031581/
>>
>> I've build and smoke tested without any problems on Linux/x86_64 and PPC64, Windows/x86_64, MacOSX, Solaris/SPARC64 and AIX7PPC64.
>>
>> With these changes (and together with the changes from "8028537: PPC64: Updated jdk/test scripts to understand the AIX os and environment" and "8031134 : PPC64: implement printing on AIX") our port passes all but the following 7 jtreg regression tests on AIX (compared to the Linux/x86_64 baseline from www.java.net/download/jdk8/testresults/testresults.html):
>>
>> java/net/Inet6Address/B6558853.java
>> java/nio/channels/AsynchronousChannelGroup/Basic.java (sporadically)
>> java/nio/channels/AsynchronousChannelGroup/GroupOfOne.java
>> java/nio/channels/AsynchronousChannelGroup/Unbounded.java (sporadically)
>> java/nio/channels/Selector/RacyDeregister.java
>> sun/security/krb5/auto/Unreachable.java (only on IPv6)
>>
>> Thank you and best regards,
>> Volker
>>
>>
>> Following a detailed description of the various changes:
>> src/share/native/java/util/zip/zip_util.c
>> src/share/native/sun/management/DiagnosticCommandImpl.c
>>
>> According to ISO C it is perfectly legal for malloc to return zero if called with a zero argument. Fix various places where malloc can potentially correctly return zero because it was called with a zero argument.
>> Also fixed DiagnosticCommandImpl.c to include stdlib.h. This only fixes a compiler warning on Linux, but on AIX it prevents a VM crash later on because the return value of malloc() will be casted to int which is especially bad if that pointer was bigger than 32-bit.
>> make/CompileJavaClasses.gmk
>>
>> Also use PollingWatchService on AIX.
>> make/lib/NioLibraries.gmk
>> src/aix/native/sun/nio/ch/AixNativeThread.c
>>
>> Put the implementation for the native methods of NativeThread into AixNativeThread.c on AIX.
>> src/solaris/native/sun/nio/ch/PollArrayWrapper.c
>> src/solaris/native/sun/nio/ch/Net.c
>> src/aix/classes/sun/nio/ch/AixPollPort.java
>> src/aix/native/sun/nio/ch/AixPollPort.c
>> src/aix/native/java/net/aix_close.c
>>
>> On AIX, the constants used for the polling events (i.e. POLLIN, POLLOUT, ...) are defined to different values than on other operating systems. The problem is however, that these constants are hardcoded as public final static members of various, shared Java classes. We therefore have to map them from Java to native every time before calling one of the native poll functions and back to Java after the call on AIX in order to get the right semantics.
>> src/share/classes/java/nio/file/CopyMoveHelper.java
>>
>> As discussed on the core-libs mailing list (see http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-December/024119.html) it is not necessary to call Files.getFileAttributeView() with any linkOptions because at that place we've already checked that the target file can not be a symbolic link. This change makes the implementation more robust on platforms which support symbolic links but do not support the O_NOFOLLOW flag to the open system call. It also makes the JDK pass the demo/zipfs/basic.sh test on AIX.
>> src/share/classes/sun/nio/cs/ext/ExtendedCharsets.java
>>
>> Support "compound text" on AIX in the same way like on other Unix platforms.
>> src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider
>>
>> Define the correct attach provider for AIX.
>> src/solaris/native/java/net/net_util_md.h
>> src/solaris/native/sun/nio/ch/FileDispatcherImpl.c
>> src/solaris/native/sun/nio/ch/ServerSocketChannelImpl.c
>>
>> AIX needs a workaround for I/O cancellation (see: http://publib.boulder.ibm.com/infocenter/pseries/v5r3/index.jsp?topic=/com.ibm.aix.basetechref/doc/basetrf1/close.htm). "..The close() subroutine is blocked until all subroutines which use the file descriptor return to usr space. For example, when a thread is calling close and another thread is calling select with the same file descriptor, the close subroutine does not return until the select call returns...". To fix this problem, we have to use the various NET_ wrappers which are declared in net_util_md.h and defined in aix_close.c and we also need some additional wrappers for fcntl(), read() and write() on AIX.
>> While the current solution isn't really nice because it introduces some more AIX-specifc sections in shared code, I think it is the best way to go for JDK 8 because it imposes the smallest possible changes and risks for the existing platforms. I'm ready to change the code to unconditionally use the wrappers for all platforms and implement the wrappers empty on platforms which don't need any wrapping. I think it would also be nice to clean up the names (e.g. NET_Read() is currently a wrapper for recv() and the NET_ prefix is probably not appropriate any more so maybe change it to something like IO_). But again, I'll prefer to keep that as a follow up change for JDK9.
>> Calling fsync() on a "read-only" file descriptor on AIX will result in an error (i.e. "EBADF: The FileDescriptor parameter is not a valid file descriptor open for writing."). To prevent this error we have to query if the corresponding file descriptor is writeable. Notice that at that point we can not access the writable attribute of the corresponding file channel so we have to use fcntl().
>> src/solaris/classes/java/lang/UNIXProcess.java.aix
>>
>> On AIX the implementation is especially tricky, because the close() system call will block if another thread is at the same time blocked in a file operation (e.g. 'read()') on the same file descriptor. We therefore combine the AIX ProcessPipeInputStream implemenatation with the DeferredCloseInputStream approach used on Solaris (see UNIXProcess.java.solaris). This means that every potentially blocking operation on the file descriptor increments a counter before it is executed and decrements it once it finishes. The 'close()' operation will only be executed if there are no pending operations. Otherwise it is deferred after the last pending operation has finished.
>> src/share/transport/socket/socketTransport.c
>>
>> On AIX we have to call shutdown() on a socket descriptor before closing it, otherwise the close() call may be blocked. This is the same problem as described before. Unfortunately the JDI framework doesn't use the same IO wrappers like other class library components so we can not easily use the NET_ abstractions from aix_close.c here.
>> Without this small change all JDI regression tests will fail on AIX because of the way how the tests act as a "debugger" which launches another VM (the "debugge") which connects itself back to the debugger. In this scenario the "debugge" can not shut down itself because one thread will always be blocked in the close() call on one of the communication sockets.
>> src/solaris/native/java/net/NetworkInterface.c
>>
>> Set the scope identifier for IPv6 addresses on AIX.
>> src/solaris/native/java/net/net_util_md.c
>>
>> It turns out that we do not always have to replace SO_REUSEADDR on AIX by SO_REUSEPORT. Instead we can simply use the same approach like BSD and only use SO_REUSEPORT additionally, if several datagram sockets try to bind to the same port.
>> Also fixed a comment and removed unused local variables.
>> Fixed the obviously inverted assignment newTime = prevTime; which should read prevTime = newTime;. Otherwise prevTime will never change and the timeout will be potential reached too fast.
>> src/solaris/native/sun/management/OperatingSystemImpl.c
>>
>> AIX does not understand /proc/self so we have to query the real process ID to access the proc file system.
>> src/solaris/native/sun/nio/ch/DatagramChannelImpl.c
>>
>> On AIX, connect() may legally return EAFNOSUPPORT if called on a socket with the address family set to AF_UNSPEC.
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20140115/7a8e80d0/attachment-0001.html
More information about the serviceability-dev
mailing list