Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
Could you please help us with this? Is it a legal scenario to
redefine shared classes?
Sorry for the delay in responding. I was on vacation.
Yes, redefining a shared class is supported via either JVM/TI or
via java.lang.instrument. There should be existing tests that
verify this behavior, but you may have discovered a corner case
by accident.
Dan
On 11/27/13 8:13 AM, Leonid Mesnik wrote:
I think test should test instrumentation of custom, JDK, JDK shared
classes as a part of scenario.
This is how tools could use this mechanism. May be I am wrong.
Also I found
* JDK-5002268 https://bugs.openjdk.java.net/browse/JDK-5002268
Allow class sharing use with RedefineClasses
which allow to redefine of classes when class sharing is used. I don't
exactly know should it work
for j.l.instrument as well as for JVMTI
Dan
Could you please help us with this? Is it a legal scenario to redefine
shared classes?
Leonid
On 11/27/2013 03:46 PM, Mattias Tobiasson wrote:
According to the test documentation and bug references the test verifies the manifest
attribute Can-Redefine-Classes.
The test does not mention shared class archive or -Xshare.
But maybe the test has found a problem accidentally...
Mattias
- Original Message -
From:david.hol...@oracle.com
To:mattias.tobias...@oracle.com
Cc:serviceability-dev@openjdk.java.net
Sent: Wednesday, November 27, 2013 12:21:50 PM GMT +01:00 Amsterdam / Berlin /
Bern / Rome / Stockholm / Vienna
Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
On 27/11/2013 8:46 PM, Mattias Tobiasson wrote:
Hi, I now have a reproducer for this test that fails every time.
I just need to verify that it really is a test bug and not a product bug.
This is a summary of the test:
1. It loads a javaagent jar that implements ClassFileTransformer.
2. The agent loads class java.math.BigInteger and expects a callback to
ClassFileTransformer.transform(). But it sometimes does not get that callback.
I believe the reason for the intermittent failures are that the jvm flag
-Xshare have different default values on different servers.
-Xshare:auto will default to off for server compiler, but on for client
When the reproducer is run with flag -Xshare:off it passes every time.
When run with -Xshare:auto it fails every time.
When the reproducer is changed to use a dummy class (RedefineDummy) instead of
BigInteger, then the test passes every time no matter what value -Xshare has.
My proposed fix for the bug is to use a dummy class instead of BigInteger. I
just need to verify that it is ok that ClassFileTransformer.transform() is NOT
called for BigInteger when -Xshare is enabled.
If the whole point of the test is to transform a file from the shared
archive then changing to a class not in the archive will defeat that. So
you need to verify what the intent of the test is.
David
The reproducer is shown below. It loads the javaagent into its own vm so I
don't need to set up multiple jvms.
public class TestRedefine implements ClassFileTransformer {
public static void agentmain(String agentArgs, Instrumentation inst)
throws Throwable {
try {
inst.addTransformer(new TestRedefine());
ClassLoader.getSystemClassLoader().loadClass(java.math.BigInteger);
if (!isTransformCalled) {
throw new Exception(transform() NOT called for +
targetName);
}
} catch (Throwable t) {
t.printStackTrace();
throw t;
}
}
public byte[] transform(ClassLoader loader,
String className,
Class? classBeingRedefined,
ProtectionDomain protectionDomain,
byte[] classfileBuffer) {
System.out.println(transform called: + className);
if (className.equals(java/math/BigInteger)) {
isTransformCalled = true;
}
return null;
}
public static void main(String args[]) throws Exception {
String pid = getProcessId();
try {
VirtualMachine vm = VirtualMachine.attach(pid);
vm.loadAgent(TestRedefine.jar);
} catch (Exception e) {
e.printStackTrace();
throw e;
}
}
... more code ...
}
- Original Message -
From:leonid.mes...@oracle.com
To:mattias.tobias...@oracle.com
Cc:serviceability-dev@openjdk.java.net
Sent: Wednesday, November 27, 2013 9:19:37 AM GMT +01:00 Amsterdam / Berlin /
Bern / Rome / Stockholm / Vienna
Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
Hi
Generally I am ok with your comments and fix. However you still needed
to get official review.
The only question is about failure related from retransformation of
j.l.BigInteger with CDS.
Could it be JDK issue? In this case it would be better to file
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
Isn’t redefining (when you start with a fresh byte code copy) different from
retransforming where you are depending on the VM to provide you with the byte
code through the class_file_load_hook? I think what we have here is the latter
and that no class_file_load_hooks are executed for shared classes? I need to
dig deep into what capabilities the j.l.instrument native agent is actually
asking for.
/Staffan
On 4 dec 2013, at 19:57, Daniel D. Daugherty daniel.daughe...@oracle.com
wrote:
Could you please help us with this? Is it a legal scenario to redefine
shared classes?
Sorry for the delay in responding. I was on vacation.
Yes, redefining a shared class is supported via either JVM/TI or
via java.lang.instrument. There should be existing tests that
verify this behavior, but you may have discovered a corner case
by accident.
Dan
On 11/27/13 8:13 AM, Leonid Mesnik wrote:
I think test should test instrumentation of custom, JDK, JDK shared classes
as a part of scenario.
This is how tools could use this mechanism. May be I am wrong.
Also I found
JDK-5002268 Allow class sharing use with RedefineClasses
which allow to redefine of classes when class sharing is used. I don't
exactly know should it work
for j.l.instrument as well as for JVMTI
Dan
Could you please help us with this? Is it a legal scenario to redefine
shared classes?
Leonid
On 11/27/2013 03:46 PM, Mattias Tobiasson wrote:
According to the test documentation and bug references the test verifies
the manifest attribute Can-Redefine-Classes.
The test does not mention shared class archive or -Xshare.
But maybe the test has found a problem accidentally...
Mattias
- Original Message -
From: david.hol...@oracle.com
To: mattias.tobias...@oracle.com
Cc: serviceability-dev@openjdk.java.net
Sent: Wednesday, November 27, 2013 12:21:50 PM GMT +01:00 Amsterdam /
Berlin / Bern / Rome / Stockholm / Vienna
Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails
intermittently
On 27/11/2013 8:46 PM, Mattias Tobiasson wrote:
Hi, I now have a reproducer for this test that fails every time.
I just need to verify that it really is a test bug and not a product bug.
This is a summary of the test:
1. It loads a javaagent jar that implements ClassFileTransformer.
2. The agent loads class java.math.BigInteger and expects a callback to
ClassFileTransformer.transform(). But it sometimes does not get that
callback.
I believe the reason for the intermittent failures are that the jvm flag
-Xshare have different default values on different servers.
-Xshare:auto will default to off for server compiler, but on for client
When the reproducer is run with flag -Xshare:off it passes every time.
When run with -Xshare:auto it fails every time.
When the reproducer is changed to use a dummy class (RedefineDummy)
instead of BigInteger, then the test passes every time no matter what
value -Xshare has.
My proposed fix for the bug is to use a dummy class instead of BigInteger.
I just need to verify that it is ok that ClassFileTransformer.transform()
is NOT called for BigInteger when -Xshare is enabled.
If the whole point of the test is to transform a file from the shared
archive then changing to a class not in the archive will defeat that. So
you need to verify what the intent of the test is.
David
The reproducer is shown below. It loads the javaagent into its own vm so I
don't need to set up multiple jvms.
public class TestRedefine implements ClassFileTransformer {
public static void agentmain(String agentArgs, Instrumentation inst)
throws Throwable {
try {
inst.addTransformer(new TestRedefine());
ClassLoader.getSystemClassLoader().loadClass(java.math.BigInteger);
if (!isTransformCalled) {
throw new Exception(transform() NOT called for +
targetName);
}
} catch (Throwable t) {
t.printStackTrace();
throw t;
}
}
public byte[] transform(ClassLoader loader,
String className,
Class? classBeingRedefined,
ProtectionDomain protectionDomain,
byte[] classfileBuffer) {
System.out.println(transform called: + className);
if (className.equals(java/math/BigInteger)) {
isTransformCalled = true;
}
return null;
}
public static void main(String args[]) throws Exception {
String pid = getProcessId();
try {
VirtualMachine vm = VirtualMachine.attach(pid);
vm.loadAgent(TestRedefine.jar);
} catch (Exception e) {
e.printStackTrace();
throw e;
}
}
... more code ...
}
- Original Message -
From: leonid.mes
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
Hi, I have updated the patch after the review. webrev: http://cr.openjdk.java.net/~ykantser/6461635/webrev.04/ Changes: 1. Removed BasicTests.sh from ProblemList 2. Use sun.tools.jar.Main to create jars. 3. Application.java uses try-with-resources. 4. Changed name of nested test class from Impl to TestMain. Mattias - Original Message - From: mattias.tobias...@oracle.com To: alan.bate...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Monday, December 2, 2013 2:59:16 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently Thanks for the feedback! Q: Do you plan to remove the no-longer-exists BasicTests.sh from the exclude list (jdk/test/ProblemList.txt), I didn't see this in webrev.03. A: Yes, I forgot about ProblemList.txt. I will remove the test from the list. Q: I wasn't aware of JDKToolLauncher and just wonder if it is necessary for it to launch the jar command. In other areas then we invoke jar directly via sun.tools.jar.Main and this avoids needing to launch a new VM. A: Ok, I will use sun.tools.jar insted. Q: One thing about the shell tests is that launch the target VM with the VM options that are specified to jtreg via -vmoption. Are these used now? (if not maybe this is a separate task for the test infrastructure library). A: Both -vmoptions and -javaoptions are added by function ProcessTools.executeTestJvm() Q: Application.java - looks like this could use try-with-resources. A: Yes. I will fix that. Q: jdk/test/sun/tools/jstatd/JstatdTest.java - is this meant to be in the webrev? A: JstatdTest.java is not directly related to this fix, but it needed to be updated because of changes in the common testlibrary class ProcessThread.java. JstatdTest expected ProcessThread to verify that exit code was 0 and stdout was empty. Since ProcessThread is a common testlibrary function that more tests will use, I do not want that function to verify exit code and stdout. That check has been moved from ProcessThread to JstatdTest. JstatdTest was the only test that used ProcessThread, so no other tests needed to be updated because of this change in ProcessThread. Q: class Impl. If you are looking for an alternative name then something like TestMain might be more obvious. A: In version webrev.03, the Impl classes are no longer used. Instead there are only 1 java file that contains both setup and test code. The test code is in a nested class. There are very small changes between webrev.00 and webrev.03 other than this merge of the two java classes for each test. Mattias - Original Message - From: alan.bate...@oracle.com To: mattias.tobias...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Monday, December 2, 2013 2:21:32 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently On 02/12/2013 10:51, Mattias Tobiasson wrote: Hi, Could someone please review this? Leonid Mesnik has looked at it and he is ok with the changes. Unfortunately he is not a reviewer. I am waiting with another test fix that is dependent on this fix. webrev: http://cr.openjdk.java.net/~miauno/6461635/webrev.03/ I went through the changes in the webrev and it looks very good and nice to see more shell tests going away. Do you plan to remove the no-longer-exists BasicTests.sh from the exclude list (jdk/test/ProblemList.txt), I didn't see this in webrev.03. I wasn't aware of JDKToolLauncher and just wonder if it is necessary for it to launch the jar command. In other areas then we invoke jar directly via sun.tools.jar.Main and this avoids needing to launch a new VM. One thing about the shell tests is that launch the target VM with the VM options that are specified to jtreg via -vmoption. Are these used now? (if not maybe this is a separate task for the test infrastructure library). Some small comments as I read through it: Application.java - looks like this could use try-with-resources. jdk/test/sun/tools/jstatd/JstatdTest.java - is this meant to be in the webrev? class Impl. If you are looking for an alternative name then something like TestMain might be more obvious. Otherwise, as I said, this is good work and nice to see BasicTests running again (as he has been excluded for a long time, I think mostly due to the redefine/Xshare issue). -Alan
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
On 03/12/2013 11:53, Mattias Tobiasson wrote: Hi, I have updated the patch after the review. webrev: http://cr.openjdk.java.net/~ykantser/6461635/webrev.04/ Changes: 1. Removed BasicTests.sh from ProblemList 2. Use sun.tools.jar.Main to create jars. 3. Application.java uses try-with-resources. 4. Changed name of nested test class from Impl to TestMain. I looks at the updated webrev and the changes look good. One other opportunity for try-with-resources is in RunnerUtil.readFile, I didn't notice this in previous round. Better still would be use Files.readAllBytes to read the file in one call, or readAllLines if the file may be composed of several lines. This is just a minor point so no need to re-generate the webrev if you change anything here. I see Staffan is going to sponsor this for you. -Alan
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
On 03/12/2013 16:17, Mattias Tobiasson wrote: Can I set you as a reviewer of this patch as it is now? If so, I will ask Staffan to submit it. That's fine, push away (and I said in one of the other mails, this is good work and nice to see this test liberated as a non-shell test). -Alan
RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
Hi, Could someone please review this? Leonid Mesnik has looked at it and he is ok with the changes. Unfortunately he is not a reviewer. I am waiting with another test fix that is dependent on this fix. Thanks, Mattias - Original Message - From: mattias.tobias...@oracle.com To: serviceability-dev@openjdk.java.net Cc: leonid.mes...@oracle.com Sent: Thursday, November 28, 2013 1:56:47 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently The change has been updated after review. Current summary of changes: 1. To avoid the complication of shared archives, the test now uses a dummy class for the retransform test. A new bug has been created for the possible problem of transforming classes loaded from archive: https://bugs.openjdk.java.net/browse/JDK-8029334 2. Removed all bash scripts. 3. Merged the setup bash script and the java test code for each test to a single java file. The old java test code is unchanged, it has just been moved to static nested class in the new java setup file. 4. Added some utility functions to jdk/testlibrary. webrev: http://cr.openjdk.java.net/~miauno/6461635/webrev.03/ bug: https://bugs.openjdk.java.net/browse/JDK-6461635 Mattias - Original Message - From: leonid.mes...@oracle.com To: mattias.tobias...@oracle.com Cc: daniel.daughe...@oracle.com, serviceability-dev@openjdk.java.net, david.hol...@oracle.com Sent: Thursday, November 28, 2013 11:02:30 AM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently Mattias On 11/28/2013 01:48 PM, Mattias Tobiasson wrote: Is it ok if I check in the current test as it is now, and add a new bug for the (possible) problem of retransforming classes from the shared archive? It is ok if you check current test which redefine non-shared class and add new bug about shared transformation. The usage of non-shared class is cleaner rather disabling sharing explicitly. However you still need a review for this fix. Reasons for why I want to check in the current test as it is are: The purpose of the current test is not about the shared archive and is already quite large. A simpler reproducer could be used for the new bug. I think it would be better to add more testcases into this test or more tests with similar but different scenarios. The new tests could be part of fix of new issue which you are going to file. Leonid The new bug may not even be a bug, but working as expected. I want someone else to look at the bug before adding a new test. This fix also contains updates for the testlibrary that are needed for other tests. This bug was reported in 2006, so it is not a regression in jdk8. Mattias - Original Message - From: leonid.mes...@oracle.com To: mattias.tobias...@oracle.com , david.hol...@oracle.com Cc: serviceability-dev@openjdk.java.net , daniel.daughe...@oracle.com Sent: Wednesday, November 27, 2013 4:15:09 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently I think test should test instrumentation of custom, JDK, JDK shared classes as a part of scenario. This is how tools could use this mechanism. May be I am wrong. Also I found • JDK-5002268 Allow class sharing use with RedefineClasses which allow to redefine of classes when class sharing is used. I don't exactly know should it work for j.l.instrument as well as for JVMTI Dan Could you please help us with this? Is it a legal scenario to redefine shared classes? Leonid On 11/27/2013 03:46 PM, Mattias Tobiasson wrote: According to the test documentation and bug references the test verifies the manifest attribute Can-Redefine-Classes. The test does not mention shared class archive or -Xshare. But maybe the test has found a problem accidentally... Mattias - Original Message - From: david.hol...@oracle.com To: mattias.tobias...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Wednesday, November 27, 2013 12:21:50 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently On 27/11/2013 8:46 PM, Mattias Tobiasson wrote: Hi, I now have a reproducer for this test that fails every time. I just need to verify that it really is a test bug and not a product bug. This is a summary of the test: 1. It loads a javaagent jar that implements ClassFileTransformer. 2. The agent loads class java.math.BigInteger and expects a callback to ClassFileTransformer.transform(). But it sometimes does not get that callback. I believe the reason for the intermittent failures are that the jvm flag -Xshare have different default values on different servers. -Xshare:auto will default
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
On 02/12/2013 10:51, Mattias Tobiasson wrote: Hi, Could someone please review this? Leonid Mesnik has looked at it and he is ok with the changes. Unfortunately he is not a reviewer. I am waiting with another test fix that is dependent on this fix. webrev: http://cr.openjdk.java.net/~miauno/6461635/webrev.03/ I went through the changes in the webrev and it looks very good and nice to see more shell tests going away. Do you plan to remove the no-longer-exists BasicTests.sh from the exclude list (jdk/test/ProblemList.txt), I didn't see this in webrev.03. I wasn't aware of JDKToolLauncher and just wonder if it is necessary for it to launch the jar command. In other areas then we invoke jar directly via sun.tools.jar.Main and this avoids needing to launch a new VM. One thing about the shell tests is that launch the target VM with the VM options that are specified to jtreg via -vmoption. Are these used now? (if not maybe this is a separate task for the test infrastructure library). Some small comments as I read through it: Application.java - looks like this could use try-with-resources. jdk/test/sun/tools/jstatd/JstatdTest.java - is this meant to be in the webrev? class Impl. If you are looking for an alternative name then something like TestMain might be more obvious. Otherwise, as I said, this is good work and nice to see BasicTests running again (as he has been excluded for a long time, I think mostly due to the redefine/Xshare issue). -Alan
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
Thanks for the feedback! Q: Do you plan to remove the no-longer-exists BasicTests.sh from the exclude list (jdk/test/ProblemList.txt), I didn't see this in webrev.03. A: Yes, I forgot about ProblemList.txt. I will remove the test from the list. Q: I wasn't aware of JDKToolLauncher and just wonder if it is necessary for it to launch the jar command. In other areas then we invoke jar directly via sun.tools.jar.Main and this avoids needing to launch a new VM. A: Ok, I will use sun.tools.jar insted. Q: One thing about the shell tests is that launch the target VM with the VM options that are specified to jtreg via -vmoption. Are these used now? (if not maybe this is a separate task for the test infrastructure library). A: Both -vmoptions and -javaoptions are added by function ProcessTools.executeTestJvm() Q: Application.java - looks like this could use try-with-resources. A: Yes. I will fix that. Q: jdk/test/sun/tools/jstatd/JstatdTest.java - is this meant to be in the webrev? A: JstatdTest.java is not directly related to this fix, but it needed to be updated because of changes in the common testlibrary class ProcessThread.java. JstatdTest expected ProcessThread to verify that exit code was 0 and stdout was empty. Since ProcessThread is a common testlibrary function that more tests will use, I do not want that function to verify exit code and stdout. That check has been moved from ProcessThread to JstatdTest. JstatdTest was the only test that used ProcessThread, so no other tests needed to be updated because of this change in ProcessThread. Q: class Impl. If you are looking for an alternative name then something like TestMain might be more obvious. A: In version webrev.03, the Impl classes are no longer used. Instead there are only 1 java file that contains both setup and test code. The test code is in a nested class. There are very small changes between webrev.00 and webrev.03 other than this merge of the two java classes for each test. Mattias - Original Message - From: alan.bate...@oracle.com To: mattias.tobias...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Monday, December 2, 2013 2:21:32 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently On 02/12/2013 10:51, Mattias Tobiasson wrote: Hi, Could someone please review this? Leonid Mesnik has looked at it and he is ok with the changes. Unfortunately he is not a reviewer. I am waiting with another test fix that is dependent on this fix. webrev: http://cr.openjdk.java.net/~miauno/6461635/webrev.03/ I went through the changes in the webrev and it looks very good and nice to see more shell tests going away. Do you plan to remove the no-longer-exists BasicTests.sh from the exclude list (jdk/test/ProblemList.txt), I didn't see this in webrev.03. I wasn't aware of JDKToolLauncher and just wonder if it is necessary for it to launch the jar command. In other areas then we invoke jar directly via sun.tools.jar.Main and this avoids needing to launch a new VM. One thing about the shell tests is that launch the target VM with the VM options that are specified to jtreg via -vmoption. Are these used now? (if not maybe this is a separate task for the test infrastructure library). Some small comments as I read through it: Application.java - looks like this could use try-with-resources. jdk/test/sun/tools/jstatd/JstatdTest.java - is this meant to be in the webrev? class Impl. If you are looking for an alternative name then something like TestMain might be more obvious. Otherwise, as I said, this is good work and nice to see BasicTests running again (as he has been excluded for a long time, I think mostly due to the redefine/Xshare issue). -Alan
Fwd: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
About the last question: class Impl. If you are looking for an alternative name then something like TestMain might be more obvious. At first I thought it was e reference to the BasicTestImpl.java in the first version. I now realize that it was the name of the nested class. I had even forgotten they were called Impl. :) I will change them to TestMain - Forwarded Message - From: mattias.tobias...@oracle.com To: alan.bate...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Monday, December 2, 2013 2:59:16 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently Thanks for the feedback! Q: Do you plan to remove the no-longer-exists BasicTests.sh from the exclude list (jdk/test/ProblemList.txt), I didn't see this in webrev.03. A: Yes, I forgot about ProblemList.txt. I will remove the test from the list. Q: I wasn't aware of JDKToolLauncher and just wonder if it is necessary for it to launch the jar command. In other areas then we invoke jar directly via sun.tools.jar.Main and this avoids needing to launch a new VM. A: Ok, I will use sun.tools.jar insted. Q: One thing about the shell tests is that launch the target VM with the VM options that are specified to jtreg via -vmoption. Are these used now? (if not maybe this is a separate task for the test infrastructure library). A: Both -vmoptions and -javaoptions are added by function ProcessTools.executeTestJvm() Q: Application.java - looks like this could use try-with-resources. A: Yes. I will fix that. Q: jdk/test/sun/tools/jstatd/JstatdTest.java - is this meant to be in the webrev? A: JstatdTest.java is not directly related to this fix, but it needed to be updated because of changes in the common testlibrary class ProcessThread.java. JstatdTest expected ProcessThread to verify that exit code was 0 and stdout was empty. Since ProcessThread is a common testlibrary function that more tests will use, I do not want that function to verify exit code and stdout. That check has been moved from ProcessThread to JstatdTest. JstatdTest was the only test that used ProcessThread, so no other tests needed to be updated because of this change in ProcessThread. Q: class Impl. If you are looking for an alternative name then something like TestMain might be more obvious. A: In version webrev.03, the Impl classes are no longer used. Instead there are only 1 java file that contains both setup and test code. The test code is in a nested class. There are very small changes between webrev.00 and webrev.03 other than this merge of the two java classes for each test. Mattias - Original Message - From: alan.bate...@oracle.com To: mattias.tobias...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Monday, December 2, 2013 2:21:32 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently On 02/12/2013 10:51, Mattias Tobiasson wrote: Hi, Could someone please review this? Leonid Mesnik has looked at it and he is ok with the changes. Unfortunately he is not a reviewer. I am waiting with another test fix that is dependent on this fix. webrev: http://cr.openjdk.java.net/~miauno/6461635/webrev.03/ I went through the changes in the webrev and it looks very good and nice to see more shell tests going away. Do you plan to remove the no-longer-exists BasicTests.sh from the exclude list (jdk/test/ProblemList.txt), I didn't see this in webrev.03. I wasn't aware of JDKToolLauncher and just wonder if it is necessary for it to launch the jar command. In other areas then we invoke jar directly via sun.tools.jar.Main and this avoids needing to launch a new VM. One thing about the shell tests is that launch the target VM with the VM options that are specified to jtreg via -vmoption. Are these used now? (if not maybe this is a separate task for the test infrastructure library). Some small comments as I read through it: Application.java - looks like this could use try-with-resources. jdk/test/sun/tools/jstatd/JstatdTest.java - is this meant to be in the webrev? class Impl. If you are looking for an alternative name then something like TestMain might be more obvious. Otherwise, as I said, this is good work and nice to see BasicTests running again (as he has been excluded for a long time, I think mostly due to the redefine/Xshare issue). -Alan
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
Is it ok if I check in the current test as it is now, and add a new bug for the
(possible) problem of retransforming classes from the shared archive?
Reasons for why I want to check in the current test as it is are:
The purpose of the current test is not about the shared archive and is already
quite large. A simpler reproducer could be used for the new bug.
The new bug may not even be a bug, but working as expected. I want someone else
to look at the bug before adding a new test.
This fix also contains updates for the testlibrary that are needed for other
tests.
This bug was reported in 2006, so it is not a regression in jdk8.
Mattias
- Original Message -
From: leonid.mes...@oracle.com
To: mattias.tobias...@oracle.com, david.hol...@oracle.com
Cc: serviceability-dev@openjdk.java.net, daniel.daughe...@oracle.com
Sent: Wednesday, November 27, 2013 4:15:09 PM GMT +01:00 Amsterdam / Berlin /
Bern / Rome / Stockholm / Vienna
Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails
intermittently
I think test should test instrumentation of custom, JDK, JDK shared classes as
a part of scenario.
This is how tools could use this mechanism. May be I am wrong.
Also I found
• JDK-5002268 Allow class sharing use with RedefineClasses
which allow to redefine of classes when class sharing is used. I don't exactly
know should it work
for j.l.instrument as well as for JVMTI
Dan
Could you please help us with this? Is it a legal scenario to redefine shared
classes?
Leonid
On 11/27/2013 03:46 PM, Mattias Tobiasson wrote:
According to the test documentation and bug references the test verifies the
manifest attribute Can-Redefine-Classes.
The test does not mention shared class archive or -Xshare.
But maybe the test has found a problem accidentally...
Mattias
- Original Message -
From: david.hol...@oracle.com To: mattias.tobias...@oracle.com Cc:
serviceability-dev@openjdk.java.net Sent: Wednesday, November 27, 2013 12:21:50
PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna
Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
On 27/11/2013 8:46 PM, Mattias Tobiasson wrote:
Hi, I now have a reproducer for this test that fails every time.
I just need to verify that it really is a test bug and not a product bug.
This is a summary of the test:
1. It loads a javaagent jar that implements ClassFileTransformer.
2. The agent loads class java.math.BigInteger and expects a callback to
ClassFileTransformer.transform(). But it sometimes does not get that callback.
I believe the reason for the intermittent failures are that the jvm flag
-Xshare have different default values on different servers. -Xshare:auto will
default to off for server compiler, but on for client
When the reproducer is run with flag -Xshare:off it passes every time.
When run with -Xshare:auto it fails every time.
When the reproducer is changed to use a dummy class (RedefineDummy) instead of
BigInteger, then the test passes every time no matter what value -Xshare has.
My proposed fix for the bug is to use a dummy class instead of BigInteger. I
just need to verify that it is ok that ClassFileTransformer.transform() is NOT
called for BigInteger when -Xshare is enabled. If the whole point of the test
is to transform a file from the shared
archive then changing to a class not in the archive will defeat that. So
you need to verify what the intent of the test is.
David
The reproducer is shown below. It loads the javaagent into its own vm so I
don't need to set up multiple jvms.
public class TestRedefine implements ClassFileTransformer {
public static void agentmain(String agentArgs, Instrumentation inst)
throws Throwable {
try {
inst.addTransformer(new TestRedefine());
ClassLoader.getSystemClassLoader().loadClass(java.math.BigInteger);
if (!isTransformCalled) {
throw new Exception(transform() NOT called for +
targetName);
}
} catch (Throwable t) {
t.printStackTrace();
throw t;
}
}
public byte[] transform(ClassLoader loader,
String className,
Class? classBeingRedefined,
ProtectionDomain protectionDomain,
byte[] classfileBuffer) {
System.out.println(transform called: + className);
if (className.equals(java/math/BigInteger)) {
isTransformCalled = true;
}
return null;
}
public static void main(String args[]) throws Exception {
String pid = getProcessId();
try {
VirtualMachine vm = VirtualMachine.attach(pid);
vm.loadAgent(TestRedefine.jar);
} catch (Exception e) {
e.printStackTrace();
throw e;
}
}
... more code
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
Mattias
On 11/28/2013 01:48 PM, Mattias Tobiasson wrote:
Is it ok if I check in the current test as it is now, and add a new
bug for the (possible) problem of retransforming classes from the
shared archive?
It is ok if you check current test which redefine non-shared class and
add new bug about shared transformation.
The usage of non-shared class is cleaner rather disabling sharing
explicitly.
However you still need a review for this fix.
Reasons for why I want to check in the current test as it is are:
The purpose of the current test is not about the shared archive and is
already quite large. A simpler reproducer could be used for the new bug.
I think it would be better to add more testcases into this test or more
tests with similar but different scenarios.
The new tests could be part of fix of new issue which you are going to
file.
Leonid
The new bug may not even be a bug, but working as expected. I want
someone else to look at the bug before adding a new test.
This fix also contains updates for the testlibrary that are needed for
other tests.
This bug was reported in 2006, so it is not a regression in jdk8.
Mattias
- Original Message -
From: leonid.mes...@oracle.com
To: mattias.tobias...@oracle.com, david.hol...@oracle.com
Cc: serviceability-dev@openjdk.java.net, daniel.daughe...@oracle.com
Sent: Wednesday, November 27, 2013 4:15:09 PM GMT +01:00 Amsterdam /
Berlin / Bern / Rome / Stockholm / Vienna
Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails
intermittently
I think test should test instrumentation of custom, JDK, JDK shared
classes as a part of scenario.
This is how tools could use this mechanism. May be I am wrong.
Also I found
* JDK-5002268 https://bugs.openjdk.java.net/browse/JDK-5002268
Allow class sharing use with RedefineClasses
which allow to redefine of classes when class sharing is used. I don't
exactly know should it work
for j.l.instrument as well as for JVMTI
Dan
Could you please help us with this? Is it a legal scenario to redefine
shared classes?
Leonid
On 11/27/2013 03:46 PM, Mattias Tobiasson wrote:
According to the test documentation and bug references the test verifies the manifest
attribute Can-Redefine-Classes.
The test does not mention shared class archive or -Xshare.
But maybe the test has found a problem accidentally...
Mattias
- Original Message -
From:david.hol...@oracle.com
To:mattias.tobias...@oracle.com
Cc:serviceability-dev@openjdk.java.net
Sent: Wednesday, November 27, 2013 12:21:50 PM GMT +01:00 Amsterdam /
Berlin / Bern / Rome / Stockholm / Vienna
Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails
intermittently
On 27/11/2013 8:46 PM, Mattias Tobiasson wrote:
Hi, I now have a reproducer for this test that fails every time.
I just need to verify that it really is a test bug and not a product
bug.
This is a summary of the test:
1. It loads a javaagent jar that implements ClassFileTransformer.
2. The agent loads class java.math.BigInteger and expects a callback to
ClassFileTransformer.transform(). But it sometimes does not get that callback.
I believe the reason for the intermittent failures are that the jvm
flag -Xshare have different default values on different servers.
-Xshare:auto will default to off for server compiler, but on for client
When the reproducer is run with flag -Xshare:off it passes every time.
When run with -Xshare:auto it fails every time.
When the reproducer is changed to use a dummy class (RedefineDummy)
instead of BigInteger, then the test passes every time no matter what value
-Xshare has.
My proposed fix for the bug is to use a dummy class instead of
BigInteger. I just need to verify that it is ok that
ClassFileTransformer.transform() is NOT called for BigInteger when -Xshare is
enabled.
If the whole point of the test is to transform a file from the shared
archive then changing to a class not in the archive will defeat that. So
you need to verify what the intent of the test is.
David
The reproducer is shown below. It loads the javaagent into its own vm
so I don't need to set up multiple jvms.
public class TestRedefine implements ClassFileTransformer {
public static void agentmain(String agentArgs, Instrumentation
inst) throws Throwable {
try {
inst.addTransformer(new TestRedefine());
ClassLoader.getSystemClassLoader().loadClass(java.math.BigInteger);
if (!isTransformCalled) {
throw new Exception(transform() NOT called for +
targetName);
}
} catch (Throwable t) {
t.printStackTrace();
throw t;
}
}
public byte
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
The change has been updated after review. Current summary of changes: 1. To avoid the complication of shared archives, the test now uses a dummy class for the retransform test. A new bug has been created for the possible problem of transforming classes loaded from archive: https://bugs.openjdk.java.net/browse/JDK-8029334 2. Removed all bash scripts. 3. Merged the setup bash script and the java test code for each test to a single java file. The old java test code is unchanged, it has just been moved to static nested class in the new java setup file. 4. Added some utility functions to jdk/testlibrary. webrev: http://cr.openjdk.java.net/~miauno/6461635/webrev.03/ bug: https://bugs.openjdk.java.net/browse/JDK-6461635 Mattias - Original Message - From: leonid.mes...@oracle.com To: mattias.tobias...@oracle.com Cc: daniel.daughe...@oracle.com, serviceability-dev@openjdk.java.net, david.hol...@oracle.com Sent: Thursday, November 28, 2013 11:02:30 AM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently Mattias On 11/28/2013 01:48 PM, Mattias Tobiasson wrote: Is it ok if I check in the current test as it is now, and add a new bug for the (possible) problem of retransforming classes from the shared archive? It is ok if you check current test which redefine non-shared class and add new bug about shared transformation. The usage of non-shared class is cleaner rather disabling sharing explicitly. However you still need a review for this fix. Reasons for why I want to check in the current test as it is are: The purpose of the current test is not about the shared archive and is already quite large. A simpler reproducer could be used for the new bug. I think it would be better to add more testcases into this test or more tests with similar but different scenarios. The new tests could be part of fix of new issue which you are going to file. Leonid The new bug may not even be a bug, but working as expected. I want someone else to look at the bug before adding a new test. This fix also contains updates for the testlibrary that are needed for other tests. This bug was reported in 2006, so it is not a regression in jdk8. Mattias - Original Message - From: leonid.mes...@oracle.com To: mattias.tobias...@oracle.com , david.hol...@oracle.com Cc: serviceability-dev@openjdk.java.net , daniel.daughe...@oracle.com Sent: Wednesday, November 27, 2013 4:15:09 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently I think test should test instrumentation of custom, JDK, JDK shared classes as a part of scenario. This is how tools could use this mechanism. May be I am wrong. Also I found • JDK-5002268 Allow class sharing use with RedefineClasses which allow to redefine of classes when class sharing is used. I don't exactly know should it work for j.l.instrument as well as for JVMTI Dan Could you please help us with this? Is it a legal scenario to redefine shared classes? Leonid On 11/27/2013 03:46 PM, Mattias Tobiasson wrote: According to the test documentation and bug references the test verifies the manifest attribute Can-Redefine-Classes. The test does not mention shared class archive or -Xshare. But maybe the test has found a problem accidentally... Mattias - Original Message - From: david.hol...@oracle.com To: mattias.tobias...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Wednesday, November 27, 2013 12:21:50 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently On 27/11/2013 8:46 PM, Mattias Tobiasson wrote: Hi, I now have a reproducer for this test that fails every time. I just need to verify that it really is a test bug and not a product bug. This is a summary of the test: 1. It loads a javaagent jar that implements ClassFileTransformer. 2. The agent loads class java.math.BigInteger and expects a callback to ClassFileTransformer.transform(). But it sometimes does not get that callback. I believe the reason for the intermittent failures are that the jvm flag -Xshare have different default values on different servers. -Xshare:auto will default to off for server compiler, but on for client When the reproducer is run with flag -Xshare:off it passes every time. When run with -Xshare:auto it fails every time. When the reproducer is changed to use a dummy class (RedefineDummy) instead of BigInteger, then the test passes every time no matter what value -Xshare has. My proposed fix for the bug is to use a dummy class instead of BigInteger. I just need to verify that it is ok that ClassFileTransformer.transform() is NOT called for BigInteger when -Xshare is enabled. If the whole point of the test
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
Hi
Generally I am ok with your comments and fix. However you still needed
to get official review.
The only question is about failure related from retransformation of
j.l.BigInteger with CDS.
Could it be JDK issue? In this case it would be better to file it as a
part of fix.
I understand that using of CDS and instrumentation of JDK classes is not
related with this test
and your fix make it cleaner. However could you please verify that we
don't have JDK issue here.
see other comments inline
On 11/26/2013 08:35 PM, Mattias Tobiasson wrote:
Hi,
Thanks for the review. I have summarized the questions and answers:
Q: What is the reason of this intermittent failure?
A: The test expects the callback ClassFileTransformer.transform() to be called when it
loads a new class. That function does not seem to be called when class sharing is enabled
and a previous test has called -Xshare:dump. I am not really sure how
ClassFileTransformer works together with class sharing.
I got the idea for -Xshare:dump from this bug:
https://bugs.openjdk.java.net/browse/JDK-6571866
I have verified on jprt that it fails without that flag. I have not been able
to reproduce a failure when -Xshare:off is set.
Having thought more about the problem, I would like to use another fix. The
test currently uses java.lang.BigInteger for the transform test. I want to
change that and instead use my own class (RedefineDummy.java). Since loading my
own class is not affected by -Xshare, I do not need to set the flag.
Q: You used output in finally which is not initialized properly in the case
of Exception. Could we get another uncaught exception in finally?
A: output is only sent to getProcessLog() to get a log message. That function can handle
null values, which it just logs as null.
Thank you for explanation.
Q: In getCommandLine(), should we also trim cmd to remove last ?
A: Yes. I will fix that. I will also add a check if ProcessBuilder is null.
Ok
Q: waitForJvmPid(String key) throws Throwable. Why throw throwable?
A: I think there are so many things that can go wrong when starting a separate
java process, that I do not want to check for expected errors. I also don't
know how a test writer should handle different kind of exceptions from this
function. Any exception is logged in the sub function.
Q: Should tryFindJvmPid(String key) be private? Are we going to use it in tests
or only waitForJvmPid?
A: Yes, that function may also be used by tests. It is currently not used by
any test, but it might be useful.
Ok
Q: Timeouts in function waitForJvmPid(String key)?
A: I definitely agree of the problem. The reason for not adding a timeout
parameter to the function is that I do not want the tests to be responsible for
setting hard coded timeouts. Timeouts should be handled by the framework. I
know there are plans of adding better process handling to jtreg (with
ProcessHandler). After that jtreg should be better at handling timeouts.
I will add a flush() to stdout to make sure we log before we wait.
Hope we will update jtreg before get into this issue :)
Leonid
Mattias
- Original Message -
From: leonid.mes...@oracle.com
To: mattias.tobias...@oracle.com, serviceability-dev@openjdk.java.net
Sent: Monday, November 25, 2013 6:26:27 PM GMT +01:00 Amsterdam / Berlin / Bern
/ Rome / Stockholm / Vienna
Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
Hi
I have a couple of high-level questions.
On 11/25/2013 08:28 PM, Mattias Tobiasson wrote:
Hi,
The test has been updated after the first review.
The two java files for each test has been merged to a single file.
Updated summary of changes:
1. The real test bug fix is to add flag -Xshare:off when starting the
Application instance. Without that flag, the test for ClassFileTransformer in
RedefineAgent.java fails intermittently. The flag is added in function startApplication() in
RunnerUtil.java.
What is the reason of this intermittent failure?
2. Removed all bash scripts.
Great!
3. Merged the setup bash script and the java test code for each test to a
single java file. The old java test code is unchanged, it has just been moved
to static nested class in the new java setup file.
4. Added some utility functions to jdk/testlibrary.
Here some comments about library code.
http://cr.openjdk.java.net/~miauno/6461635/webrev.01/jdk/test/lib/testlibrary/jdk/testlibrary/ProcessThread.java.udiff.html
+try {
+output = new OutputAnalyzer(this.process);
+} catch (Throwable t) {
+String name = Thread.currentThread().getName();
+System.out.println(String.format(ProcessThread[%s] failed:
%s, name, t.toString()));
+throw t;
+} finally {
+String logMsg = ProcessTools.getProcessLog(processBuilder,
output);
+System.out.println(logMsg);
+}
You used output in finally which is not initialized
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
On 27/11/2013 8:46 PM, Mattias Tobiasson wrote:
Hi, I now have a reproducer for this test that fails every time.
I just need to verify that it really is a test bug and not a product bug.
This is a summary of the test:
1. It loads a javaagent jar that implements ClassFileTransformer.
2. The agent loads class java.math.BigInteger and expects a callback to
ClassFileTransformer.transform(). But it sometimes does not get that callback.
I believe the reason for the intermittent failures are that the jvm flag
-Xshare have different default values on different servers.
-Xshare:auto will default to off for server compiler, but on for client
When the reproducer is run with flag -Xshare:off it passes every time.
When run with -Xshare:auto it fails every time.
When the reproducer is changed to use a dummy class (RedefineDummy) instead of
BigInteger, then the test passes every time no matter what value -Xshare has.
My proposed fix for the bug is to use a dummy class instead of BigInteger. I
just need to verify that it is ok that ClassFileTransformer.transform() is NOT
called for BigInteger when -Xshare is enabled.
If the whole point of the test is to transform a file from the shared
archive then changing to a class not in the archive will defeat that. So
you need to verify what the intent of the test is.
David
The reproducer is shown below. It loads the javaagent into its own vm so I
don't need to set up multiple jvms.
public class TestRedefine implements ClassFileTransformer {
public static void agentmain(String agentArgs, Instrumentation inst)
throws Throwable {
try {
inst.addTransformer(new TestRedefine());
ClassLoader.getSystemClassLoader().loadClass(java.math.BigInteger);
if (!isTransformCalled) {
throw new Exception(transform() NOT called for +
targetName);
}
} catch (Throwable t) {
t.printStackTrace();
throw t;
}
}
public byte[] transform(ClassLoader loader,
String className,
Class? classBeingRedefined,
ProtectionDomain protectionDomain,
byte[] classfileBuffer) {
System.out.println(transform called: + className);
if (className.equals(java/math/BigInteger)) {
isTransformCalled = true;
}
return null;
}
public static void main(String args[]) throws Exception {
String pid = getProcessId();
try {
VirtualMachine vm = VirtualMachine.attach(pid);
vm.loadAgent(TestRedefine.jar);
} catch (Exception e) {
e.printStackTrace();
throw e;
}
}
... more code ...
}
- Original Message -
From: leonid.mes...@oracle.com
To: mattias.tobias...@oracle.com
Cc: serviceability-dev@openjdk.java.net
Sent: Wednesday, November 27, 2013 9:19:37 AM GMT +01:00 Amsterdam / Berlin /
Bern / Rome / Stockholm / Vienna
Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
Hi
Generally I am ok with your comments and fix. However you still needed
to get official review.
The only question is about failure related from retransformation of
j.l.BigInteger with CDS.
Could it be JDK issue? In this case it would be better to file it as a
part of fix.
I understand that using of CDS and instrumentation of JDK classes is not
related with this test
and your fix make it cleaner. However could you please verify that we
don't have JDK issue here.
see other comments inline
On 11/26/2013 08:35 PM, Mattias Tobiasson wrote:
Hi,
Thanks for the review. I have summarized the questions and answers:
Q: What is the reason of this intermittent failure?
A: The test expects the callback ClassFileTransformer.transform() to be called when it
loads a new class. That function does not seem to be called when class sharing is enabled
and a previous test has called -Xshare:dump. I am not really sure how
ClassFileTransformer works together with class sharing.
I got the idea for -Xshare:dump from this bug:
https://bugs.openjdk.java.net/browse/JDK-6571866
I have verified on jprt that it fails without that flag. I have not been able
to reproduce a failure when -Xshare:off is set.
Having thought more about the problem, I would like to use another fix. The
test currently uses java.lang.BigInteger for the transform test. I want to
change that and instead use my own class (RedefineDummy.java). Since loading my
own class is not affected by -Xshare, I do not need to set the flag.
Q: You used output in finally which is not initialized properly in the case
of Exception. Could we get another uncaught exception in finally?
A: output is only sent to getProcessLog() to get a log message. That function can handle
null values, which it just logs as null.
Thank you for explanation.
Q: In getCommandLine
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
On 27 nov 2013, at 12:21, David Holmes david.hol...@oracle.com wrote:
On 27/11/2013 8:46 PM, Mattias Tobiasson wrote:
Hi, I now have a reproducer for this test that fails every time.
I just need to verify that it really is a test bug and not a product bug.
This is a summary of the test:
1. It loads a javaagent jar that implements ClassFileTransformer.
2. The agent loads class java.math.BigInteger and expects a callback to
ClassFileTransformer.transform(). But it sometimes does not get that
callback.
I believe the reason for the intermittent failures are that the jvm flag
-Xshare have different default values on different servers.
-Xshare:auto will default to off for server compiler, but on for client
When the reproducer is run with flag -Xshare:off it passes every time.
When run with -Xshare:auto it fails every time.
When the reproducer is changed to use a dummy class (RedefineDummy) instead
of BigInteger, then the test passes every time no matter what value -Xshare
has.
My proposed fix for the bug is to use a dummy class instead of BigInteger. I
just need to verify that it is ok that ClassFileTransformer.transform() is
NOT called for BigInteger when -Xshare is enabled.
If the whole point of the test is to transform a file from the shared archive
then changing to a class not in the archive will defeat that. So you need to
verify what the intent of the test is.
There is nothing in the test that expects class data sharing to be tested. The
test is a basic verification of the attach mechanism.
As far as I can tell, It is expected that ClassFileTransformer.transform() is
not called for classes in the shared archive (BigInteger) in this case. Thus, I
think it is a good idea to either use a different class or to explicitly
disable class data sharing (simpler).
/Staffan
David
The reproducer is shown below. It loads the javaagent into its own vm so I
don't need to set up multiple jvms.
public class TestRedefine implements ClassFileTransformer {
public static void agentmain(String agentArgs, Instrumentation inst)
throws Throwable {
try {
inst.addTransformer(new TestRedefine());
ClassLoader.getSystemClassLoader().loadClass(java.math.BigInteger);
if (!isTransformCalled) {
throw new Exception(transform() NOT called for +
targetName);
}
} catch (Throwable t) {
t.printStackTrace();
throw t;
}
}
public byte[] transform(ClassLoader loader,
String className,
Class? classBeingRedefined,
ProtectionDomain protectionDomain,
byte[] classfileBuffer) {
System.out.println(transform called: + className);
if (className.equals(java/math/BigInteger)) {
isTransformCalled = true;
}
return null;
}
public static void main(String args[]) throws Exception {
String pid = getProcessId();
try {
VirtualMachine vm = VirtualMachine.attach(pid);
vm.loadAgent(TestRedefine.jar);
} catch (Exception e) {
e.printStackTrace();
throw e;
}
}
... more code ...
}
- Original Message -
From: leonid.mes...@oracle.com
To: mattias.tobias...@oracle.com
Cc: serviceability-dev@openjdk.java.net
Sent: Wednesday, November 27, 2013 9:19:37 AM GMT +01:00 Amsterdam / Berlin
/ Bern / Rome / Stockholm / Vienna
Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails
intermittently
Hi
Generally I am ok with your comments and fix. However you still needed
to get official review.
The only question is about failure related from retransformation of
j.l.BigInteger with CDS.
Could it be JDK issue? In this case it would be better to file it as a
part of fix.
I understand that using of CDS and instrumentation of JDK classes is not
related with this test
and your fix make it cleaner. However could you please verify that we
don't have JDK issue here.
see other comments inline
On 11/26/2013 08:35 PM, Mattias Tobiasson wrote:
Hi,
Thanks for the review. I have summarized the questions and answers:
Q: What is the reason of this intermittent failure?
A: The test expects the callback ClassFileTransformer.transform() to be
called when it loads a new class. That function does not seem to be called
when class sharing is enabled and a previous test has called
-Xshare:dump. I am not really sure how ClassFileTransformer works
together with class sharing.
I got the idea for -Xshare:dump from this bug:
https://bugs.openjdk.java.net/browse/JDK-6571866
I have verified on jprt that it fails without that flag. I have not been
able to reproduce a failure when -Xshare:off is set.
Having thought more about the problem, I would like to use
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
According to the test documentation and bug references the test verifies the
manifest attribute Can-Redefine-Classes.
The test does not mention shared class archive or -Xshare.
But maybe the test has found a problem accidentally...
Mattias
- Original Message -
From: david.hol...@oracle.com
To: mattias.tobias...@oracle.com
Cc: serviceability-dev@openjdk.java.net
Sent: Wednesday, November 27, 2013 12:21:50 PM GMT +01:00 Amsterdam / Berlin /
Bern / Rome / Stockholm / Vienna
Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
On 27/11/2013 8:46 PM, Mattias Tobiasson wrote:
Hi, I now have a reproducer for this test that fails every time.
I just need to verify that it really is a test bug and not a product bug.
This is a summary of the test:
1. It loads a javaagent jar that implements ClassFileTransformer.
2. The agent loads class java.math.BigInteger and expects a callback to
ClassFileTransformer.transform(). But it sometimes does not get that callback.
I believe the reason for the intermittent failures are that the jvm flag
-Xshare have different default values on different servers.
-Xshare:auto will default to off for server compiler, but on for client
When the reproducer is run with flag -Xshare:off it passes every time.
When run with -Xshare:auto it fails every time.
When the reproducer is changed to use a dummy class (RedefineDummy) instead
of BigInteger, then the test passes every time no matter what value -Xshare
has.
My proposed fix for the bug is to use a dummy class instead of BigInteger. I
just need to verify that it is ok that ClassFileTransformer.transform() is
NOT called for BigInteger when -Xshare is enabled.
If the whole point of the test is to transform a file from the shared
archive then changing to a class not in the archive will defeat that. So
you need to verify what the intent of the test is.
David
The reproducer is shown below. It loads the javaagent into its own vm so I
don't need to set up multiple jvms.
public class TestRedefine implements ClassFileTransformer {
public static void agentmain(String agentArgs, Instrumentation inst)
throws Throwable {
try {
inst.addTransformer(new TestRedefine());
ClassLoader.getSystemClassLoader().loadClass(java.math.BigInteger);
if (!isTransformCalled) {
throw new Exception(transform() NOT called for +
targetName);
}
} catch (Throwable t) {
t.printStackTrace();
throw t;
}
}
public byte[] transform(ClassLoader loader,
String className,
Class? classBeingRedefined,
ProtectionDomain protectionDomain,
byte[] classfileBuffer) {
System.out.println(transform called: + className);
if (className.equals(java/math/BigInteger)) {
isTransformCalled = true;
}
return null;
}
public static void main(String args[]) throws Exception {
String pid = getProcessId();
try {
VirtualMachine vm = VirtualMachine.attach(pid);
vm.loadAgent(TestRedefine.jar);
} catch (Exception e) {
e.printStackTrace();
throw e;
}
}
... more code ...
}
- Original Message -
From: leonid.mes...@oracle.com
To: mattias.tobias...@oracle.com
Cc: serviceability-dev@openjdk.java.net
Sent: Wednesday, November 27, 2013 9:19:37 AM GMT +01:00 Amsterdam / Berlin /
Bern / Rome / Stockholm / Vienna
Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails
intermittently
Hi
Generally I am ok with your comments and fix. However you still needed
to get official review.
The only question is about failure related from retransformation of
j.l.BigInteger with CDS.
Could it be JDK issue? In this case it would be better to file it as a
part of fix.
I understand that using of CDS and instrumentation of JDK classes is not
related with this test
and your fix make it cleaner. However could you please verify that we
don't have JDK issue here.
see other comments inline
On 11/26/2013 08:35 PM, Mattias Tobiasson wrote:
Hi,
Thanks for the review. I have summarized the questions and answers:
Q: What is the reason of this intermittent failure?
A: The test expects the callback ClassFileTransformer.transform() to be
called when it loads a new class. That function does not seem to be called
when class sharing is enabled and a previous test has called -Xshare:dump.
I am not really sure how ClassFileTransformer works together with class
sharing.
I got the idea for -Xshare:dump from this bug:
https://bugs.openjdk.java.net/browse/JDK-6571866
I have verified on jprt that it fails without that flag. I have not been
able to reproduce a failure when
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
Hi,
Thanks for the review. I have summarized the questions and answers:
Q: What is the reason of this intermittent failure?
A: The test expects the callback ClassFileTransformer.transform() to be called
when it loads a new class. That function does not seem to be called when class
sharing is enabled and a previous test has called -Xshare:dump. I am not
really sure how ClassFileTransformer works together with class sharing.
I got the idea for -Xshare:dump from this bug:
https://bugs.openjdk.java.net/browse/JDK-6571866
I have verified on jprt that it fails without that flag. I have not been able
to reproduce a failure when -Xshare:off is set.
Having thought more about the problem, I would like to use another fix. The
test currently uses java.lang.BigInteger for the transform test. I want to
change that and instead use my own class (RedefineDummy.java). Since loading my
own class is not affected by -Xshare, I do not need to set the flag.
Q: You used output in finally which is not initialized properly in the case
of Exception. Could we get another uncaught exception in finally?
A: output is only sent to getProcessLog() to get a log message. That function
can handle null values, which it just logs as null.
Q: In getCommandLine(), should we also trim cmd to remove last ?
A: Yes. I will fix that. I will also add a check if ProcessBuilder is null.
Q: waitForJvmPid(String key) throws Throwable. Why throw throwable?
A: I think there are so many things that can go wrong when starting a separate
java process, that I do not want to check for expected errors. I also don't
know how a test writer should handle different kind of exceptions from this
function. Any exception is logged in the sub function.
Q: Should tryFindJvmPid(String key) be private? Are we going to use it in tests
or only waitForJvmPid?
A: Yes, that function may also be used by tests. It is currently not used by
any test, but it might be useful.
Q: Timeouts in function waitForJvmPid(String key)?
A: I definitely agree of the problem. The reason for not adding a timeout
parameter to the function is that I do not want the tests to be responsible for
setting hard coded timeouts. Timeouts should be handled by the framework. I
know there are plans of adding better process handling to jtreg (with
ProcessHandler). After that jtreg should be better at handling timeouts.
I will add a flush() to stdout to make sure we log before we wait.
Mattias
- Original Message -
From: leonid.mes...@oracle.com
To: mattias.tobias...@oracle.com, serviceability-dev@openjdk.java.net
Sent: Monday, November 25, 2013 6:26:27 PM GMT +01:00 Amsterdam / Berlin / Bern
/ Rome / Stockholm / Vienna
Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
Hi
I have a couple of high-level questions.
On 11/25/2013 08:28 PM, Mattias Tobiasson wrote:
Hi,
The test has been updated after the first review.
The two java files for each test has been merged to a single file.
Updated summary of changes:
1. The real test bug fix is to add flag -Xshare:off when starting the
Application instance. Without that flag, the test for ClassFileTransformer
in RedefineAgent.java fails intermittently. The flag is added in function
startApplication() in RunnerUtil.java.
What is the reason of this intermittent failure?
2. Removed all bash scripts.
Great!
3. Merged the setup bash script and the java test code for each test to a
single java file. The old java test code is unchanged, it has just been moved
to static nested class in the new java setup file.
4. Added some utility functions to jdk/testlibrary.
Here some comments about library code.
http://cr.openjdk.java.net/~miauno/6461635/webrev.01/jdk/test/lib/testlibrary/jdk/testlibrary/ProcessThread.java.udiff.html
+try {
+output = new OutputAnalyzer(this.process);
+} catch (Throwable t) {
+String name = Thread.currentThread().getName();
+System.out.println(String.format(ProcessThread[%s] failed:
%s, name, t.toString()));
+throw t;
+} finally {
+String logMsg = ProcessTools.getProcessLog(processBuilder,
output);
+System.out.println(logMsg);
+}
You used output in finally which is not initialized properly in the case
of Exception.
Could we get another uncaught exception in finally?
http://cr.openjdk.java.net/~miauno/6461635/webrev.01/jdk/test/lib/testlibrary/jdk/testlibrary/ProcessTools.java.udiff.html
1) Same in as previous in executeProcess method.
2) getCommandLine (...)
+/**
+ * @return The full command line for the ProcessBuilder.
+ */
+public static String getCommandLine(ProcessBuilder pb) {
+StringBuilder cmd = new StringBuilder();
+for (String s : pb.command()) {
+cmd.append(s).append( );
+}
+return cmd.toString();
+}
Should we also trim cmd to remove last
RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
Hi, The test has been updated after the first review. The two java files for each test has been merged to a single file. Updated summary of changes: 1. The real test bug fix is to add flag -Xshare:off when starting the Application instance. Without that flag, the test for ClassFileTransformer in RedefineAgent.java fails intermittently. The flag is added in function startApplication() in RunnerUtil.java. 2. Removed all bash scripts. 3. Merged the setup bash script and the java test code for each test to a single java file. The old java test code is unchanged, it has just been moved to static nested class in the new java setup file. 4. Added some utility functions to jdk/testlibrary. 5. Moved exit code check from the common utility function in ProcessThread.java to the test JstatdTest.java. The check is moved to the test because other tests in the future may have other expected exit codes. Webrev: http://cr.openjdk.java.net/~miauno/6461635/webrev.01/ Bug: https://bugs.openjdk.java.net/browse/JDK-6461635 Mattias - Original Message - From: mattias.tobias...@oracle.com To: mikael.a...@oracle.com Cc: serviceability-dev@openjdk.java.net, alan.bate...@oracle.com Sent: Thursday, November 21, 2013 9:22:11 AM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently I agree that merging both files in a single file may be good. The main reason why I did not do that is that I wanted to keep the history of the existing test. If I copy the test to a new file, all history of the code is lost. But this test bug was reported 8 years ago, and I don't believe much has changed in the code since then. So keeping the history may not be that important. Mattias - Original Message - From: mikael.a...@oracle.com To: mattias.tobias...@oracle.com, alan.bate...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Wednesday, November 20, 2013 4:11:45 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently How about defining the class that you want to attach to as a static inner class to the actual test? That would give you only one file, but with two classes, each with its own main method and clear correlation between them. Mikael On 2013-11-20 15:41, Mattias Tobiasson wrote: Hi, Each test requires 2 files, the actual test code and a helper file. The helper file will launch a separate java process (called Application), and then start the actual test. The actual test will then attach to the Application. For example: PermissionTests.sh: Helper file that will launch Application instance and then start PermissionTest.java PermissionTest.java: The actual test code that attaches to the Application. It is the PermissionTests.sh that is started by jtreg (contains the @test-tag). When I port PermissionTest.sh to java I would get 2 files called PermissionTest.java, so some name change is required. I could have kept the old PermissionTest.java unchanged, but then I would need another name for the prevoius PermissionTest.sh. And I wanted a clean Test name for the file containing the @test-tag. I used these names: TestPermission.java: Helper file. TestPermissionImpl.java: Actual test code. I am still new to adding tests for open jdk. I am happy to change the names if they do not follow the naming convention. Mattias - Original Message - From: alan.bate...@oracle.com To: mattias.tobias...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Wednesday, November 20, 2013 2:50:52 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently Out of curiosity, what is the reason for the rename? I ask because we use Basic and similar names in many areas. Also anything in the test tree should be a test. -Alan. On 20/11/2013 13:47, Mattias Tobiasson wrote: Hi, Could you please review this fix. Summary of changes: 1. The real test bug fix is to add flag -Xshare:off when starting the Application instance. Without that flag, the test for ClassFileTransformer in RedefineAgent.java fails intermittently. The flag is added in function startApplication() in RunnerUtil.java. 2. Ported the following bash scripts to java: BasicTests.sh - TestBasic.java PermissionTests.sh - TestPermission.java ProviderTests.sh - TestProvider.java 3. Renamed the java test code to avoid name clash with new java classes ported from bash script: BasicsTest.java - TestBasicImpl.java PermissionTest.java - TestPermissionImpl.java ProviderTest.java - TestProviderImpl.java 4. Added some utility functions to jdk/testlibrary. 5. Moved exit code check from the common utility function in ProcessThread.java to the test JstatdTest.java. The check is moved to the test because
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
Hi
I have a couple of high-level questions.
On 11/25/2013 08:28 PM, Mattias Tobiasson wrote:
Hi,
The test has been updated after the first review.
The two java files for each test has been merged to a single file.
Updated summary of changes:
1. The real test bug fix is to add flag -Xshare:off when starting the
Application instance. Without that flag, the test for ClassFileTransformer in
RedefineAgent.java fails intermittently. The flag is added in function startApplication() in
RunnerUtil.java.
What is the reason of this intermittent failure?
2. Removed all bash scripts.
Great!
3. Merged the setup bash script and the java test code for each test to a
single java file. The old java test code is unchanged, it has just been moved
to static nested class in the new java setup file.
4. Added some utility functions to jdk/testlibrary.
Here some comments about library code.
http://cr.openjdk.java.net/~miauno/6461635/webrev.01/jdk/test/lib/testlibrary/jdk/testlibrary/ProcessThread.java.udiff.html
+try {
+output = new OutputAnalyzer(this.process);
+} catch (Throwable t) {
+String name = Thread.currentThread().getName();
+System.out.println(String.format(ProcessThread[%s] failed:
%s, name, t.toString()));
+throw t;
+} finally {
+String logMsg = ProcessTools.getProcessLog(processBuilder,
output);
+System.out.println(logMsg);
+}
You used output in finally which is not initialized properly in the case
of Exception.
Could we get another uncaught exception in finally?
http://cr.openjdk.java.net/~miauno/6461635/webrev.01/jdk/test/lib/testlibrary/jdk/testlibrary/ProcessTools.java.udiff.html
1) Same in as previous in executeProcess method.
2) getCommandLine (...)
+/**
+ * @return The full command line for the ProcessBuilder.
+ */
+public static String getCommandLine(ProcessBuilder pb) {
+StringBuilder cmd = new StringBuilder();
+for (String s : pb.command()) {
+cmd.append(s).append( );
+}
+return cmd.toString();
+}
Should we also trim cmd to remove last ?
http://cr.openjdk.java.net/~miauno/6461635/webrev.01/jdk/test/lib/testlibrary/jdk/testlibrary/Utils.java.udiff.html
public static int waitForJvmPid(String key) throws Throwable {
Why it throw Trowable? Could we deal with exceptions in this method and
re-throw some more meaningful exception here?
Could you force flush for out here:
System.out.println(waitForJvmPid: Waiting for key ' + key + ');
Hope it should be enough. It is scary to investigate such timeouts.
Would it be better to add some
internal timeout in testlibrary? Really jtreg doesn't kill all processes
and we have alive java processes in such case.
For samevm tests timeouts are even worse. There is no good way to kill
test in samevm.
Should be
public static int tryFindJvmPid(String key) throws Throwable {
private? Are we going to use it in tests or only waitForJvmPid?
Leonid
5. Moved exit code check from the common utility function in ProcessThread.java
to the test JstatdTest.java. The check is moved to the test because other tests
in the future may have other expected exit codes.
Webrev:
http://cr.openjdk.java.net/~miauno/6461635/webrev.01/
Bug:
https://bugs.openjdk.java.net/browse/JDK-6461635
Mattias
- Original Message -
From: mattias.tobias...@oracle.com
To: mikael.a...@oracle.com
Cc: serviceability-dev@openjdk.java.net, alan.bate...@oracle.com
Sent: Thursday, November 21, 2013 9:22:11 AM GMT +01:00 Amsterdam / Berlin /
Bern / Rome / Stockholm / Vienna
Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
I agree that merging both files in a single file may be good.
The main reason why I did not do that is that I wanted to keep the history of
the existing test. If I copy the test to a new file, all history of the code is
lost.
But this test bug was reported 8 years ago, and I don't believe much has
changed in the code since then. So keeping the history may not be that
important.
Mattias
- Original Message -
From: mikael.a...@oracle.com
To: mattias.tobias...@oracle.com, alan.bate...@oracle.com
Cc: serviceability-dev@openjdk.java.net
Sent: Wednesday, November 20, 2013 4:11:45 PM GMT +01:00 Amsterdam / Berlin /
Bern / Rome / Stockholm / Vienna
Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
How about defining the class that you want to attach to as a static
inner class to the actual test? That would give you only one file, but
with two classes, each with its own main method and clear correlation
between them.
Mikael
On 2013-11-20 15:41, Mattias Tobiasson wrote:
Hi,
Each test requires 2 files, the actual test code and a helper file.
The helper file will launch a separate java process (called Application), and
then start the actual test. The actual test
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
I agree that merging both files in a single file may be good. The main reason why I did not do that is that I wanted to keep the history of the existing test. If I copy the test to a new file, all history of the code is lost. But this test bug was reported 8 years ago, and I don't believe much has changed in the code since then. So keeping the history may not be that important. Mattias - Original Message - From: mikael.a...@oracle.com To: mattias.tobias...@oracle.com, alan.bate...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Wednesday, November 20, 2013 4:11:45 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently How about defining the class that you want to attach to as a static inner class to the actual test? That would give you only one file, but with two classes, each with its own main method and clear correlation between them. Mikael On 2013-11-20 15:41, Mattias Tobiasson wrote: Hi, Each test requires 2 files, the actual test code and a helper file. The helper file will launch a separate java process (called Application), and then start the actual test. The actual test will then attach to the Application. For example: PermissionTests.sh: Helper file that will launch Application instance and then start PermissionTest.java PermissionTest.java: The actual test code that attaches to the Application. It is the PermissionTests.sh that is started by jtreg (contains the @test-tag). When I port PermissionTest.sh to java I would get 2 files called PermissionTest.java, so some name change is required. I could have kept the old PermissionTest.java unchanged, but then I would need another name for the prevoius PermissionTest.sh. And I wanted a clean Test name for the file containing the @test-tag. I used these names: TestPermission.java: Helper file. TestPermissionImpl.java: Actual test code. I am still new to adding tests for open jdk. I am happy to change the names if they do not follow the naming convention. Mattias - Original Message - From: alan.bate...@oracle.com To: mattias.tobias...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Wednesday, November 20, 2013 2:50:52 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently Out of curiosity, what is the reason for the rename? I ask because we use Basic and similar names in many areas. Also anything in the test tree should be a test. -Alan. On 20/11/2013 13:47, Mattias Tobiasson wrote: Hi, Could you please review this fix. Summary of changes: 1. The real test bug fix is to add flag -Xshare:off when starting the Application instance. Without that flag, the test for ClassFileTransformer in RedefineAgent.java fails intermittently. The flag is added in function startApplication() in RunnerUtil.java. 2. Ported the following bash scripts to java: BasicTests.sh - TestBasic.java PermissionTests.sh - TestPermission.java ProviderTests.sh - TestProvider.java 3. Renamed the java test code to avoid name clash with new java classes ported from bash script: BasicsTest.java - TestBasicImpl.java PermissionTest.java - TestPermissionImpl.java ProviderTest.java - TestProviderImpl.java 4. Added some utility functions to jdk/testlibrary. 5. Moved exit code check from the common utility function in ProcessThread.java to the test JstatdTest.java. The check is moved to the test because other tests in the future may have other expected exit codes. Thanks, Mattias Webrev: http://cr.openjdk.java.net/~miauno/6461635/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-6461635
RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
Hi, Could you please review this fix. Summary of changes: 1. The real test bug fix is to add flag -Xshare:off when starting the Application instance. Without that flag, the test for ClassFileTransformer in RedefineAgent.java fails intermittently. The flag is added in function startApplication() in RunnerUtil.java. 2. Ported the following bash scripts to java: BasicTests.sh - TestBasic.java PermissionTests.sh - TestPermission.java ProviderTests.sh - TestProvider.java 3. Renamed the java test code to avoid name clash with new java classes ported from bash script: BasicsTest.java - TestBasicImpl.java PermissionTest.java - TestPermissionImpl.java ProviderTest.java - TestProviderImpl.java 4. Added some utility functions to jdk/testlibrary. 5. Moved exit code check from the common utility function in ProcessThread.java to the test JstatdTest.java. The check is moved to the test because other tests in the future may have other expected exit codes. Thanks, Mattias Webrev: http://cr.openjdk.java.net/~miauno/6461635/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-6461635
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
Out of curiosity, what is the reason for the rename? I ask because we use Basic and similar names in many areas. Also anything in the test tree should be a test. -Alan. On 20/11/2013 13:47, Mattias Tobiasson wrote: Hi, Could you please review this fix. Summary of changes: 1. The real test bug fix is to add flag -Xshare:off when starting the Application instance. Without that flag, the test for ClassFileTransformer in RedefineAgent.java fails intermittently. The flag is added in function startApplication() in RunnerUtil.java. 2. Ported the following bash scripts to java: BasicTests.sh - TestBasic.java PermissionTests.sh - TestPermission.java ProviderTests.sh - TestProvider.java 3. Renamed the java test code to avoid name clash with new java classes ported from bash script: BasicsTest.java - TestBasicImpl.java PermissionTest.java - TestPermissionImpl.java ProviderTest.java - TestProviderImpl.java 4. Added some utility functions to jdk/testlibrary. 5. Moved exit code check from the common utility function in ProcessThread.java to the test JstatdTest.java. The check is moved to the test because other tests in the future may have other expected exit codes. Thanks, Mattias Webrev: http://cr.openjdk.java.net/~miauno/6461635/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-6461635
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
Hi, Each test requires 2 files, the actual test code and a helper file. The helper file will launch a separate java process (called Application), and then start the actual test. The actual test will then attach to the Application. For example: PermissionTests.sh: Helper file that will launch Application instance and then start PermissionTest.java PermissionTest.java: The actual test code that attaches to the Application. It is the PermissionTests.sh that is started by jtreg (contains the @test-tag). When I port PermissionTest.sh to java I would get 2 files called PermissionTest.java, so some name change is required. I could have kept the old PermissionTest.java unchanged, but then I would need another name for the prevoius PermissionTest.sh. And I wanted a clean Test name for the file containing the @test-tag. I used these names: TestPermission.java: Helper file. TestPermissionImpl.java: Actual test code. I am still new to adding tests for open jdk. I am happy to change the names if they do not follow the naming convention. Mattias - Original Message - From: alan.bate...@oracle.com To: mattias.tobias...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Wednesday, November 20, 2013 2:50:52 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently Out of curiosity, what is the reason for the rename? I ask because we use Basic and similar names in many areas. Also anything in the test tree should be a test. -Alan. On 20/11/2013 13:47, Mattias Tobiasson wrote: Hi, Could you please review this fix. Summary of changes: 1. The real test bug fix is to add flag -Xshare:off when starting the Application instance. Without that flag, the test for ClassFileTransformer in RedefineAgent.java fails intermittently. The flag is added in function startApplication() in RunnerUtil.java. 2. Ported the following bash scripts to java: BasicTests.sh - TestBasic.java PermissionTests.sh - TestPermission.java ProviderTests.sh - TestProvider.java 3. Renamed the java test code to avoid name clash with new java classes ported from bash script: BasicsTest.java - TestBasicImpl.java PermissionTest.java - TestPermissionImpl.java ProviderTest.java - TestProviderImpl.java 4. Added some utility functions to jdk/testlibrary. 5. Moved exit code check from the common utility function in ProcessThread.java to the test JstatdTest.java. The check is moved to the test because other tests in the future may have other expected exit codes. Thanks, Mattias Webrev: http://cr.openjdk.java.net/~miauno/6461635/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-6461635
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
How about defining the class that you want to attach to as a static inner class to the actual test? That would give you only one file, but with two classes, each with its own main method and clear correlation between them. Mikael On 2013-11-20 15:41, Mattias Tobiasson wrote: Hi, Each test requires 2 files, the actual test code and a helper file. The helper file will launch a separate java process (called Application), and then start the actual test. The actual test will then attach to the Application. For example: PermissionTests.sh: Helper file that will launch Application instance and then start PermissionTest.java PermissionTest.java: The actual test code that attaches to the Application. It is the PermissionTests.sh that is started by jtreg (contains the @test-tag). When I port PermissionTest.sh to java I would get 2 files called PermissionTest.java, so some name change is required. I could have kept the old PermissionTest.java unchanged, but then I would need another name for the prevoius PermissionTest.sh. And I wanted a clean Test name for the file containing the @test-tag. I used these names: TestPermission.java: Helper file. TestPermissionImpl.java: Actual test code. I am still new to adding tests for open jdk. I am happy to change the names if they do not follow the naming convention. Mattias - Original Message - From: alan.bate...@oracle.com To: mattias.tobias...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Wednesday, November 20, 2013 2:50:52 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently Out of curiosity, what is the reason for the rename? I ask because we use Basic and similar names in many areas. Also anything in the test tree should be a test. -Alan. On 20/11/2013 13:47, Mattias Tobiasson wrote: Hi, Could you please review this fix. Summary of changes: 1. The real test bug fix is to add flag -Xshare:off when starting the Application instance. Without that flag, the test for ClassFileTransformer in RedefineAgent.java fails intermittently. The flag is added in function startApplication() in RunnerUtil.java. 2. Ported the following bash scripts to java: BasicTests.sh - TestBasic.java PermissionTests.sh - TestPermission.java ProviderTests.sh - TestProvider.java 3. Renamed the java test code to avoid name clash with new java classes ported from bash script: BasicsTest.java - TestBasicImpl.java PermissionTest.java - TestPermissionImpl.java ProviderTest.java - TestProviderImpl.java 4. Added some utility functions to jdk/testlibrary. 5. Moved exit code check from the common utility function in ProcessThread.java to the test JstatdTest.java. The check is moved to the test because other tests in the future may have other expected exit codes. Thanks, Mattias Webrev: http://cr.openjdk.java.net/~miauno/6461635/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-6461635
