Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
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. 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.simo...@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
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
On 15/01/2014 06:24, David Holmes wrote: I'm not a fan of runtime checks of this kind though if it is only a very samll number of values it might be okay. Another option would be to make those classes into templates as done with Version.java.template and substitute the right values at build time. But I'll let Alan and net-dev folk come back with their preferred technique for this. I plan to spend time on Volker's webrev later in the week (just too busy with other things right now). For the translation issue then it's an oversight in the original implementation, it just hasn't come up before (to my knowledge anyway). The simplest solution here maybe to to just move them to sun.net.ch.Net and have them initialized to their native value. In general then I'm not too concerned about that one, it's the changes to support async close on AIX that are leaping out at me. -Alan
Re: RFR (S): 7185591: jcmd-big-script.sh ERROR: could not find app's Java pid.
Staffan, thank you for pointing out performance and test.src issues! The webrev with fixes can be found here: http://cr.openjdk.java.net/~ykantser/7185591/webrev.01/ Please, see my comments in-lined. Thanks, Katja On 01/13/2014 01:19 PM, Staffan Larsen wrote: Katja, test/lib/testlibrary/jdk/testlibrary/JcmdBase.java 68 * Run jcmd standalone I think you should expand a bit on what “standalone” means here. It took me a while to understand the difference. Fixed. test/lib/testlibrary/jdk/testlibrary/OutputAnalyzer.java 423 public int indexOf(String pattern) { This seems very inefficient. Add all lines to an ArrayList and then walk through them one at a time to if it matches and then walk through them once again to find the index of that line. 469 public int shouldMatchByLine(String from, String to, String pattern) { Same inefficiency here, but worse because both asLines() and indexOf() does the same work. test/lib/testlibrary/jdk/testlibrary/Utils.java 65 public static final String TEST_SRC = System.getProperty(test.src).trim(); Fixed. I wonder if this really works. Isn’t “test.src” different for different tests? A property that jtreg changes Yes, it is different. before invoking each test? Or does this work because each test is run in a different class loader and Utils.java will exist once in each class loader? I've learned now it works because jtreg compiles all classes a test needs to a separate location. For example when I run TestJcmdDefaults there will be both TestJcmdDefaults.class and jdk/testlibrary/Utils.class under /tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/. The class loader will load Utils for TestJcmdDefaults from /tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/. ... [Loaded jdk.testlibrary.Utils from file:/tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/] ... But I think it's better to declare TEST_SRC per test precisely because it's different for different tests. /Staffan On 10 jan 2014, at 13:50, Yekaterina Kantserova yekaterina.kantser...@oracle.com wrote: Hi, Could I please have a review of this fix. In this fix I've rewritten sun/tools/jcmd/* tests in pure java to get rid of all intermittent failures depending on for example MKS or race conditions (test application has not yet started when the test start to run). Webrev: http://cr.openjdk.java.net/~ykantser/7185591/webrev.00/ Primal bug: https://bugs.openjdk.java.net/browse/JDK-7185591 Similar bugs: https://bugs.openjdk.java.net/browse/JDK-6977426 https://bugs.openjdk.java.net/browse/JDK-8020798 https://bugs.openjdk.java.net/browse/JDK-7130656 (this one is blocked by https://bugs.openjdk.java.net/browse/JDK-8031482 so far) Thanks, Katja
Re: RFR (S): 7185591: jcmd-big-script.sh ERROR: could not find app's Java pid.
Katja, test/lib/testlibrary/jdk/testlibrary/JcmdBase.java The toolArgs parameter should be renamed jcmdArgs. test/lib/testlibrary/jdk/testlibrary/OutputAnalyzer.java 423 public int indexOf(String pattern) { You don’t have to convert the List to an array before iterating through it. Use lLines.get(i).matches(…) instead. 470 public int shouldMatchByLine(String from, String to, String pattern) { This method still reads the lines into an ArrayList (at most) three times. It calls asLines() once and indexOf() twice. There could be a version of indexOf() that takes a ListString. Actually indexOf could be private and always take a ListString. Otherwise ok! /Staffan On 15 jan 2014, at 10:13, Yekaterina Kantserova yekaterina.kantser...@oracle.com wrote: Staffan, thank you for pointing out performance and test.src issues! The webrev with fixes can be found here: http://cr.openjdk.java.net/~ykantser/7185591/webrev.01/ Please, see my comments in-lined. Thanks, Katja On 01/13/2014 01:19 PM, Staffan Larsen wrote: Katja, test/lib/testlibrary/jdk/testlibrary/JcmdBase.java 68 * Run jcmd standalone I think you should expand a bit on what “standalone” means here. It took me a while to understand the difference. Fixed. test/lib/testlibrary/jdk/testlibrary/OutputAnalyzer.java 423 public int indexOf(String pattern) { This seems very inefficient. Add all lines to an ArrayList and then walk through them one at a time to if it matches and then walk through them once again to find the index of that line. 469 public int shouldMatchByLine(String from, String to, String pattern) { Same inefficiency here, but worse because both asLines() and indexOf() does the same work. test/lib/testlibrary/jdk/testlibrary/Utils.java 65 public static final String TEST_SRC = System.getProperty(test.src).trim(); Fixed. I wonder if this really works. Isn’t “test.src” different for different tests? A property that jtreg changes Yes, it is different. before invoking each test? Or does this work because each test is run in a different class loader and Utils.java will exist once in each class loader? I've learned now it works because jtreg compiles all classes a test needs to a separate location. For example when I run TestJcmdDefaults there will be both TestJcmdDefaults.class and jdk/testlibrary/Utils.class under /tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/. The class loader will load Utils for TestJcmdDefaults from /tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/. ... [Loaded jdk.testlibrary.Utils from file:/tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/] ... But I think it's better to declare TEST_SRC per test precisely because it's different for different tests. /Staffan On 10 jan 2014, at 13:50, Yekaterina Kantserova yekaterina.kantser...@oracle.com wrote: Hi, Could I please have a review of this fix. In this fix I've rewritten sun/tools/jcmd/* tests in pure java to get rid of all intermittent failures depending on for example MKS or race conditions (test application has not yet started when the test start to run). Webrev: http://cr.openjdk.java.net/~ykantser/7185591/webrev.00/ Primal bug: https://bugs.openjdk.java.net/browse/JDK-7185591 Similar bugs: https://bugs.openjdk.java.net/browse/JDK-6977426 https://bugs.openjdk.java.net/browse/JDK-8020798 https://bugs.openjdk.java.net/browse/JDK-7130656 (this one is blocked by https://bugs.openjdk.java.net/browse/JDK-8031482 so far) Thanks, Katja
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
On Wed, Jan 15, 2014 at 10:03 AM, Alan Bateman alan.bate...@oracle.comwrote: On 15/01/2014 06:24, David Holmes wrote: I'm not a fan of runtime checks of this kind though if it is only a very samll number of values it might be okay. Another option would be to make those classes into templates as done with Version.java.template and substitute the right values at build time. But I'll let Alan and net-dev folk come back with their preferred technique for this. I plan to spend time on Volker's webrev later in the week (just too busy with other things right now). For the translation issue then it's an oversight in the original implementation, it just hasn't come up before (to my knowledge anyway). The simplest solution here maybe to to just move them to sun.net.ch.Net and have them initialized to their native value. Do you mean sun.nio.ch.Net right? Do you propose to completely remove the definitions of the POLL constants from: src/share/classes/sun/nio/ch/AbstractPollArrayWrapper.java src/solaris/classes/sun/nio/ch/Port.java and replace all their usages by Net.POLL* ? In general then I'm not too concerned about that one, it's the changes to support async close on AIX that are leaping out at me. -Alan
Re: RFR (S): 7185591: jcmd-big-script.sh ERROR: could not find app's Java pid.
Staffan, The webrev with changes can be found here: http://cr.openjdk.java.net/~ykantser/7185591/webrev.02/ If it looks good, could you please be my sponsor and push the changes? The patch is attached to this mail. Thanks, Katja On 01/15/2014 11:37 AM, Staffan Larsen wrote: Katja, test/lib/testlibrary/jdk/testlibrary/JcmdBase.java The toolArgs parameter should be renamed jcmdArgs. test/lib/testlibrary/jdk/testlibrary/OutputAnalyzer.java 423 public int indexOf(String pattern) { You don’t have to convert the List to an array before iterating through it. Use lLines.get(i).matches(…) instead. 470 public int shouldMatchByLine(String from, String to, String pattern) { This method still reads the lines into an ArrayList (at most) three times. It calls asLines() once and indexOf() twice. There could be a version of indexOf() that takes a ListString. Actually indexOf could be private and always take a ListString. Otherwise ok! /Staffan On 15 jan 2014, at 10:13, Yekaterina Kantserova yekaterina.kantser...@oracle.com mailto:yekaterina.kantser...@oracle.com wrote: Staffan, thank you for pointing out performance and test.src issues! The webrev with fixes can be found here: http://cr.openjdk.java.net/~ykantser/7185591/webrev.01/ http://cr.openjdk.java.net/%7Eykantser/7185591/webrev.01/ Please, see my comments in-lined. Thanks, Katja On 01/13/2014 01:19 PM, Staffan Larsen wrote: Katja, test/lib/testlibrary/jdk/testlibrary/JcmdBase.java 68 * Run jcmd standalone I think you should expand a bit on what “standalone” means here. It took me a while to understand the difference. Fixed. test/lib/testlibrary/jdk/testlibrary/OutputAnalyzer.java 423 public int indexOf(String pattern) { This seems very inefficient. Add all lines to an ArrayList and then walk through them one at a time to if it matches and then walk through them once again to find the index of that line. 469 public int shouldMatchByLine(String from, String to, String pattern) { Same inefficiency here, but worse because both asLines() and indexOf() does the same work. test/lib/testlibrary/jdk/testlibrary/Utils.java 65 public static final String TEST_SRC = System.getProperty(test.src).trim(); Fixed. I wonder if this really works. Isn’t “test.src” different for different tests? A property that jtreg changes Yes, it is different. before invoking each test? Or does this work because each test is run in a different class loader and Utils.java will exist once in each class loader? I've learned now it works because jtreg compiles all classes a test needs to a separate location. For example when I run TestJcmdDefaults there will be both TestJcmdDefaults.class and jdk/testlibrary/Utils.class under /tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/. The class loader will load Utils for TestJcmdDefaults from /tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/. ... [Loaded jdk.testlibrary.Utils from file:/tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/] ... But I think it's better to declare TEST_SRC per test precisely because it's different for different tests. /Staffan On 10 jan 2014, at 13:50, Yekaterina Kantserova yekaterina.kantser...@oracle.com mailto:yekaterina.kantser...@oracle.com wrote: Hi, Could I please have a review of this fix. In this fix I've rewritten sun/tools/jcmd/* tests in pure java to get rid of all intermittent failures depending on for example MKS or race conditions (test application has not yet started when the test start to run). Webrev: http://cr.openjdk.java.net/~ykantser/7185591/webrev.00/ http://cr.openjdk.java.net/%7Eykantser/7185591/webrev.00/ Primal bug: https://bugs.openjdk.java.net/browse/JDK-7185591 Similar bugs: https://bugs.openjdk.java.net/browse/JDK-6977426 https://bugs.openjdk.java.net/browse/JDK-8020798 https://bugs.openjdk.java.net/browse/JDK-7130656 (this one is blocked by https://bugs.openjdk.java.net/browse/JDK-8031482 so far) Thanks, Katja # HG changeset patch # User ykantser # Date 1389791523 -3600 # Node ID 8f80cea368b700dcfba912cbc4297c4a3a8c63b0 # Parent 322a13ba1def144b685b52e7494eec45823fec31 7185591: jcmd-big-script.sh ERROR: could not find app's Java pid. Reviewed-by: egahlin, sla, jbachorik diff --git a/test/ProblemList.txt b/test/ProblemList.txt --- a/test/ProblemList.txt +++ b/test/ProblemList.txt @@ -277,3 +277,10 @@ # jdk_util + +# svc_tools + +# 8031482 +sun/tools/jcmd/TestJcmdSanity.javawindows-all + + diff --git a/test/lib/testlibrary/jdk/testlibrary/JcmdBase.java b/test/lib/testlibrary/jdk/testlibrary/JcmdBase.java --- a/test/lib/testlibrary/jdk/testlibrary/JcmdBase.java +++ b/test/lib/testlibrary/jdk/testlibrary/JcmdBase.java @@ -26,34 +26,91 @@ import java.util.Arrays; /** - * Super class for tests which need to attach jcmd
Re: Review request for 7195249: Some jtreg tests use hard coded ports
Hi, Please take a look at the new review. Webrev for jdk part: http://cr.openjdk.java.net/~anazarov/7195249/jdk/webrev.02/ Webrev for hs part: http://cr.openjdk.java.net/~anazarov/7195249/hs/webrev.02/ My answers are inline: On 08.01.2014 17:46, Staffan Larsen wrote: Hi Taras, Thanks for doing this clean up and conversion of tests into Java. Here’s a couple of comments: test/runtime/6294277/SourceDebugExtension.java: This test could be simplified by not specifying an address at all. Since the test never connects to the JVM started with -Xrunjdwp, there is no reason to specify an address. If address is unspecified (and server=y), the connector will pick an address and print it to the command line. Thus the only change that needs to be done is to remove ,address=” from the @run command. fixed test/sun/management/jmxremote/bootstrap/RmiBootstrapTest.sh: test/sun/management/jmxremote/bootstrap/RmiSslBootstrapTest.sh: These tests do not compile cleanly with an empty JTwork directory. It seems that having one @build for each class does not work well - when compiling RmiBootstrapTest.java it cannot find TestLogger. Moving all classes to one @build statement solved this problem for me. fixed test/lib/testlibrary/jdk/testlibrary/ProcessTools.java: 187 FutureVoid stdoutTask = stdout.process(); 188 FutureVoid stderrTask = stderr.process(); The stdoutTask and stderrTask variables are unused. fixed test/sun/management/jmxremote/bootstrap/RmiRegistrySslTest.java: At first I thought something was wrong with this file - the diff is very weird. Then I realized you renamed an old file and created a new file using the old name. You are right. I did it to keep the test name. test/sun/management/jmxremote/bootstrap/AbstractFilePermissionTest.java: - Is resetPasswordFilePermission() really necessary? It looks like you delete the files at the beginning of the test in any case. I think yes. n the first place, this functionality was at the old code. In the second place, a file without write permission may be a problem for a further cleanup (not by the test, for example for the tests launcher scripts etc.) - I find the names and usage of “mgmt” and “file2PermissionTest” confusing. They are both Paths. One is used directly by the sub-classes, the other has a getter method. fixed - Lines 57-58: Don’t swallow exceptions, add an ex.printStackTrace(). (Same thing for all other places where you call Integer.parseInt()) fixed test/sun/management/jmxremote/bootstrap/Dummy.java: This file is never used as far as I can see. It is used by PasswordFilePermissionTest SSLConfigFilePermissionTest via the AbstractFilePermissionTest (see the doTest method, AbstractFilePermissionTest : 162). Thanks, /Staffan On 26 dec 2013, at 14:09, taras ledkov taras.led...@oracle.com wrote: Hi, Please take a look at the review with fixed issues about trying to launch test that needs free port several times. Webrev for jdk part: http://cr.openjdk.java.net/~anazarov/7195249/jdk/webrev.01/ Webrev for hs part: http://cr.openjdk.java.net/~anazarov/7195249/hs/webrev.01/ Pay your attention to new method ProcessTools.startProcess(String, ProcessBuilder, ConsumerString) that is used to analyze all output of a sub-process. It has common part with ProcessTools.startProcess(String, ProcessBuilder, PredicateString, long, TumeUnit) that is used to determine the warm-up moment. I think the ProcessTools.startProcess(String, ProcessBuilder, PredicateString, long, TumeUnit) may be changed by adding LinePump to stderr if there is not serious reason for restricting the warm-up analysis to stdout stream. On 10.12.2013 16:16, Yekaterina Kantserova wrote: Hi, I've consulted with Serviceability engineers (add them to CC list) and they would like to see tests to solve these problem so far: 2. Implement loops in every test. Thanks, Katja On 12/09/2013 11:02 AM, Alexandre (Shura) Iline wrote: Guys. Let me try to sum up what was said before and may be suggest a compromise. 1. There is a desire to have a support port allocation on the level of a JTReg suite execution. Taras created a bug for that (https://bugs.openjdk.java.net/browse/JDK-7195249). Whether it is a test harness API or a library API does not really matter from usage point of view. 2. There is no way to make the tests absolutely stable, whatever port allocation logic is used. The best we could do is to try to perform the test logic with different ports until the test succeeds. Both arguments make sense. #2 is the ultimate answer, of course, but better be used in conjunction with a meaningful port selection algorithm. At the same time, copying a loop-until-success login from one test to another may be not the best solution. Library could help with that I believe. There only need to be an API method which takes behavior as a parameter and run it until it succeeds. Something like: public T runOnAFreePort(FunctionT, Integer)
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
Hi Staffan, thanks for the review. Please find my comments inline: On Wed, Jan 15, 2014 at 9:57 AM, Staffan Larsen staffan.lar...@oracle.comwrote: 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.simo...@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
hg: jdk8/tl/jdk: 8031502: JSR292: IncompatibleClassChangeError in LambdaForm for CharSequence.toString() method handle type converter
Changeset: 9e91793fd516 Author:vlivanov Date: 2014-01-15 20:48 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/9e91793fd516 8031502: JSR292: IncompatibleClassChangeError in LambdaForm for CharSequence.toString() method handle type converter Reviewed-by: sundar, lagergren, drchase ! src/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java + test/java/lang/invoke/ObjectMethodInInterfaceTest.java
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
On Wed, Jan 15, 2014 at 6:27 PM, Volker Simonis volker.simo...@gmail.com wrote: On Wed, Jan 15, 2014 at 5:34 PM, Volker Simonis volker.simo...@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.lar...@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: Sorry for the noise - it seems I was a little indisposed during the last mails:) So this is the simple change I'd like to propose for Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo(): @@ -117,19 +119,23 @@ return NULL; } num_commands = (*env)-GetArrayLength(env, commands); - dcmd_info_array = (dcmdInfo*) malloc(num_commands * - sizeof(dcmdInfo)); + dcmdInfoCls = (*env)-FindClass(env, + sun/management/DiagnosticCommandInfo); + result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL); + if (result == NULL) { + JNU_ThrowOutOfMemoryError(env, 0); + } + if (num_commands == 0) { + /* Handle the 'zero commands' case specially to avoid calling 'malloc()' */ + /* with a zero argument because that may legally return a NULL pointer. */ + 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); - dcmdInfoCls = (*env)-FindClass(env, - sun/management/DiagnosticCommandInfo); - result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL); - if (result == NULL) { - free(dcmd_info_array); - JNU_ThrowOutOfMemoryError(env, 0); - } for (i=0; inum_commands; i++) { args = getDiagnosticCommandArgumentInfoArray(env, (*env)-GetObjectArrayElement(env,commands,i), If the 'commands' input array is of zero length just return a zero length array. OK? dcmdInfoCls = (*env)-FindClass(env, sun/management/DiagnosticCommandInfo); num_commands = (*env)-GetArrayLength(env, commands); Sorry, of course I wanted to say if (num_commands == 0) here! 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.simo...@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,
RR(XS): JDK-8024049 com/sun/jdi/ProcessAttachTest.sh shortens 7-digit pid to 6-digit
Hi Everybody, Please review small, test-only fix. http://cr.openjdk.java.net/~dsamersoff/JDK-8024049/webrev.01/ fix summary: grep/cut chain is replaced with awk. -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
On 15 jan 2014, at 18:27, Volker Simonis volker.simo...@gmail.com wrote: On Wed, Jan 15, 2014 at 5:34 PM, Volker Simonis volker.simo...@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.lar...@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); Sorry, of course I wanted to say if (num_commands == 0) here! I understood as much :-) 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.simo...@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.
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
Yes, that looks like a good solution. /Staffan On 15 jan 2014, at 17:34, Volker Simonis volker.simo...@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.lar...@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.simo...@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
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
On 15 jan 2014, at 18:52, Volker Simonis volker.simo...@gmail.com wrote: On Wed, Jan 15, 2014 at 6:27 PM, Volker Simonis volker.simo...@gmail.com wrote: On Wed, Jan 15, 2014 at 5:34 PM, Volker Simonis volker.simo...@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.lar...@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: Sorry for the noise - it seems I was a little indisposed during the last mails:) So this is the simple change I'd like to propose for Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo(): @@ -117,19 +119,23 @@ return NULL; } num_commands = (*env)-GetArrayLength(env, commands); - dcmd_info_array = (dcmdInfo*) malloc(num_commands * - sizeof(dcmdInfo)); + dcmdInfoCls = (*env)-FindClass(env, + sun/management/DiagnosticCommandInfo); + result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL); + if (result == NULL) { + JNU_ThrowOutOfMemoryError(env, 0); + } + if (num_commands == 0) { + /* Handle the 'zero commands' case specially to avoid calling 'malloc()' */ + /* with a zero argument because that may legally return a NULL pointer. */ + 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); - dcmdInfoCls = (*env)-FindClass(env, - sun/management/DiagnosticCommandInfo); - result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL); - if (result == NULL) { - free(dcmd_info_array); - JNU_ThrowOutOfMemoryError(env, 0); - } for (i=0; inum_commands; i++) { args = getDiagnosticCommandArgumentInfoArray(env, (*env)-GetObjectArrayElement(env,commands,i), If the 'commands' input array is of zero length just return a zero length array. OK? Yes, this looks good (still :-) ) /Staffan dcmdInfoCls = (*env)-FindClass(env, sun/management/DiagnosticCommandInfo); num_commands = (*env)-GetArrayLength(env, commands); Sorry, of course I wanted to say if (num_commands == 0) here! 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.simo...@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):
Re: RR(XS): JDK-8024049 com/sun/jdi/ProcessAttachTest.sh shortens 7-digit pid to 6-digit
Looks good! Thanks, /Staffan On 15 jan 2014, at 19:23, Dmitry Samersoff dmitry.samers...@oracle.com wrote: Hi Everybody, Please review small, test-only fix. http://cr.openjdk.java.net/~dsamersoff/JDK-8024049/webrev.01/ fix summary: grep/cut chain is replaced with awk. -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
Review request for 8029378: com/sun/jdi/BadHandshakeTest.java failed with java.util.concurrent.TimeoutException
Hi, Please review this simple fix, the test needs more time to wait Phaser.awaitAdvanceInterruptibly(...). webrev: http://cr.openjdk.java.net/~sjiang/JDK-8029378/00/ bug: https://bugs.openjdk.java.net/browse/JDK-8029378 Thanks, Shanliang