Re: RFR(S): 8047368: Remove oracle.jrockit.jfr from open package.access list

2014-06-26 Thread Staffan Larsen
Looks good!

Thanks,
/Staffan

On 26 jun 2014, at 02:12, Erik Gahlin erik.gah...@oracle.com wrote:

 Hi,
 
 Could I have a review of a small fix that removes references to jfr from the 
 package.access list.
 
 Bug:
 https://bugs.openjdk.java.net/browse/JDK-8047368
 
 Webrev:
 http://cr.openjdk.java.net/~egahlin/8047368/
 
 Thanks
 Erik



Re: RFR(M): 8028474: sun/jvmstat/monitor/MonitoredVm/MonitorVmStartTerminate.sh timeout, leaves looping process behind

2014-06-26 Thread Staffan Larsen
Indentation around JavaProcess.getId() is weird.

JavaProcess.getPid/setPid/pid do not appear to be used.

JavaProcess.waitForRemoval: How about using timestamps (currentTimeMillis()) 
before the loop and for each iteration to determine if the timeout has expired 
(instead of time+=100”)?

nit: missing empty line before line 139, method releaseStarted().

/Staffan


On 26 jun 2014, at 00:56, Erik Gahlin erik.gah...@oracle.com wrote:

 It didn't work. 
 
 There's no termination notification, if I use Process#destroyForcibly(). I 
 believe the hsperfdata file is left behind so it will not be able to detect 
 that the process has died.
 
 Here is an updated version that renames some variable/methods.
 
 http://cr.openjdk.java.net/~egahlin/8028474_3/
 
 Thanks
 Erik
 
 Erik Gahlin skrev 2014-06-18 09:37:
 Didn't know about destroyForcibly(). I could try that.
 
 Erik
 
 Staffan Larsen skrev 18/06/14 09:26:
 Erik,
 
 How about using Process.destroyForcibly() as a means to terminate the 
 process instead of using files for signaling?
 
 /Staffan
  
 On 17 jun 2014, at 23:13, Erik Gahlin erik.gah...@oracle.com wrote:
 
 Yes, very weird
 
 Could have been that I needed the tools.jar in the child processes, or it 
 could have been something else. If I remember correctly, I got a CNF that 
 I didn't get with the shell script, but it was few months back.
 
 Anyway, I retried with JPRT and now it works without the shell script.
 
 http://cr.openjdk.java.net/~egahlin/8028474_2/
 
 Erik
 
 Staffan Larsen skrev 2014-06-16 13:49:
 
 On 16 jun 2014, at 12:32, Erik Gahlin erik.gah...@oracle.com wrote:
 
 Yekaterina Kantserova skrev 13/06/14 12:59:
 Erik,
 
 is there some reason why we need to keep MonitorVmStartTerminate.sh? 
 I've moved the JTreg header to MonitorVmStartTerminate.java
 Hi Katja,
 
 That's how I did the test initially, and it works locally, but I could 
 never get it to work in JPRT without the shell script. I believe the 
 tools.jar is not on the class path.
 
 That is weird. I see other tests that depend in tools.jar and they work 
 fine.
 
 /Staffan
 
 
 
 Erik
 
 /*
  * @test
  * @bug 4990825
  * @summary attach to external but local JVM processes
  * @library /lib/testlibrary
  * @build jdk.testlibrary.*
  * @run main MonitorVmStartTerminate
  */
 
 and the test works just fine.
 
 The JTreg run contains all pathes and system properties 
 MonitorVmStartTerminate.sh tries to construct:
 ${JAVA} ${TESTVMOPTS} -Dtest.jdk=${TESTJAVA} 
 -Dtest.classes=${TESTCLASSES} -classpath ${CP} MonitorVmStartTerminate
 
 See the log attached.
 
 Note @build jdk.testlibrary.* instead of @build 
 jdk.testlibrary.ProcessTools to make sure all testlibrary classes are 
 compiled 
 to the right place when running tests concurrently.
 
 Thanks,
 Katja (Not a Reviewer)
 
 
 
 On 06/12/2014 12:37 AM, Erik Gahlin wrote:
 Hi, 
 
 Could I have a review of a test that has been failing 
 intermittently. The test now uses files for synchronization 
 instead of waiting for a process that sleeps. 
 
 Webrev: 
 http://cr.openjdk.java.net/~egahlin/8028474/ 
 
 Bug: 
 https://bugs.openjdk.java.net/browse/JDK-8028474 
 
 Description: 
 
 The test starts ten Java processes, each with a unique id. 
 
  Each process creates a file named after the id and then it waits for 
  the test to remove the file, at which the Java process exits. 
 
  The processes are monitored by the test to make sure notifications 
  are sent when processes are started/terminated. 
 
  To avoid Java processes being left behind, in case of an unexpected 
  failure, shutdown hooks are registered that remove files when the 
 test 
  exits. If files are not removed, i.e. due to a JVM crash, 
  the Java processes will exit themselves after 1000 s. 
 
 Thanks 
 Erik 
 
 
 
 
 
 
 



RFR JDK-8031554: com/sun/tools/attach/BasicTests.java fails intermittently

2014-06-26 Thread shanliang

Hi,

Today ProcessTools.executeProcess has the code:
new OutputAnalyzer(pb.start());

and OutputAnalyzer constructor calls immediately:
exitValue = process.exitValue();

the test got exception because the process did not end.

So a direct solution for the test is not to call:
   ProcessTools.executeTestJvm(args);

but:
ProcessBuilder pb = 
ProcessTools.createJavaProcessBuilder(Utils.addTestJavaOpts(args));

Process process = pb.start();
process.waitFor();
OutputAnalyzer output = new OutputAnalyzer(process);

here we do waiting:
process.waitFor();
before constructing an OutputAnalyzer.

ProcessTools is a tool class we may have same issue for other tests 
using this class. So we may need to improve the test library.


bug: https://bugs.openjdk.java.net/browse/JDK-8031554
webrev: http://cr.openjdk.java.net/~sjiang/JDK-8031554/00/


Thanks,
Shanliang



Re: RFR JDK-8031554: com/sun/tools/attach/BasicTests.java fails intermittently

2014-06-26 Thread Jaroslav Bachorik

Hi Shanliang,

On 06/26/2014 03:15 PM, shanliang wrote:

Hi,

Today ProcessTools.executeProcess has the code:
 new OutputAnalyzer(pb.start());

and OutputAnalyzer constructor calls immediately:
 exitValue = process.exitValue();

the test got exception because the process did not end.


Are you sure about this?

The OutputAnalyzer constructor, before calling process.exitValue(), 
calls ProcessTools.getOutput(process) which actually does process.waitFor()


A probable explanation would be that process.waitFor() gets interrupted 
without the target process actually ending.


Either the result of ProcessTools.getOutput() should be checked for null 
to detect this situation or ProcessTools.getOutput() should throw a more 
aggressive exception when waitFor() gets interrupted.


-JB-



So a direct solution for the test is not to call:
ProcessTools.executeTestJvm(args);

but:
 ProcessBuilder pb =
ProcessTools.createJavaProcessBuilder(Utils.addTestJavaOpts(args));
 Process process = pb.start();
 process.waitFor();
 OutputAnalyzer output = new OutputAnalyzer(process);

here we do waiting:
 process.waitFor();
before constructing an OutputAnalyzer.

ProcessTools is a tool class we may have same issue for other tests
using this class. So we may need to improve the test library.

bug: https://bugs.openjdk.java.net/browse/JDK-8031554
webrev: http://cr.openjdk.java.net/~sjiang/JDK-8031554/00/


Thanks,
Shanliang





Re: RFR JDK-8031554: com/sun/tools/attach/BasicTests.java fails intermittently

2014-06-26 Thread shanliang

Jaroslav Bachorik wrote:

Hi Shanliang,

On 06/26/2014 03:15 PM, shanliang wrote:

Hi,

Today ProcessTools.executeProcess has the code:
 new OutputAnalyzer(pb.start());

and OutputAnalyzer constructor calls immediately:
 exitValue = process.exitValue();

the test got exception because the process did not end.


Are you sure about this?

The OutputAnalyzer constructor, before calling process.exitValue(), 
calls ProcessTools.getOutput(process) which actually does 
process.waitFor()

Good catch!


A probable explanation would be that process.waitFor() gets 
interrupted without the target process actually ending.


Either the result of ProcessTools.getOutput() should be checked for 
null to detect this situation or ProcessTools.getOutput() should throw 
a more aggressive exception when waitFor() gets interrupted.
It was possible beacause of an InterruptedException, but maybe a Process 
issue too.


process.waitFor() was called by the test main thread, I am wondering who 
wanted to interrupt this thread?


Not know why ProcessTools.getOutput() catches InterruptedException and 
then returns null, are there some other tests dependent to this 
behavior? otherwise better to not catch InterruptedException.


I think to keep this modification, it will allow us to get more 
information if the bug is reproduced, if getting an InterruptedException 
then we need to do more investigation for why, if no exception then we 
may rise a question to process.waitFor().


Shanliang


-JB-



So a direct solution for the test is not to call:
ProcessTools.executeTestJvm(args);

but:
 ProcessBuilder pb =
ProcessTools.createJavaProcessBuilder(Utils.addTestJavaOpts(args));
 Process process = pb.start();
 process.waitFor();
 OutputAnalyzer output = new OutputAnalyzer(process);

here we do waiting:
 process.waitFor();
before constructing an OutputAnalyzer.

ProcessTools is a tool class we may have same issue for other tests
using this class. So we may need to improve the test library.

bug: https://bugs.openjdk.java.net/browse/JDK-8031554
webrev: http://cr.openjdk.java.net/~sjiang/JDK-8031554/00/


Thanks,
Shanliang







Re: RFR (S) 8013942: JSR 292: assert(type() == T_OBJECT) failed: type check

2014-06-26 Thread Vladimir Ivanov

Looks good.

Best regards,
Vladimir Ivanov

On 6/25/14 5:57 PM, serguei.spit...@oracle.com wrote:

Please, review the fix for:
   https://bugs.openjdk.java.net/browse/JDK-8013942


Open webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2014/hotspot/8013942-JVMTI-lobj.1



Summary:

   This is a Nightly Stabilization issue.

   The problem is that the JVMTI GetLocalObject() function is hitting
the assert
   as the type of the local at current bci is not T_OBJECT as expected.
   It is because this local is alive only in a narrow scope of one
condition in the method but current bci is out of this csope.

   A fragment from the target method:

 static Class? canonicalize(Class? t, int how) {
 Class? ct; == The local
 if (t == Object.class) {
 // no change, ever
 } else if (!t.isPrimitive()) {
 switch (how) {
 case UNWRAP:
 ct = Wrapper.asPrimitiveType(t); == Initialized here
 if (ct != t) return ct;
 break;
 case RAW_RETURN:
 case ERASE:
 return Object.class;
 }
 } else if (t == void.class) {== The
GetLocalObject() is called with bci in this block
 // no change, usually
 switch (how) {
 case RAW_RETURN:
 return int.class;
 case WRAP:
 return Void.class;
 }
 } else {
 . . .

   The local 'ct' is only alive in the block of condition if
(!t.isPrimitive()).
   However, it is dead local in the next block.

   The fix suggested in the webrev above is to use the oop_mask for the
method's current bci.
   It tells when the local is dead in the scope of this bci.
   Return the JVMTI_ERROR_INVALID_SLOT error in such a case.

   The fix also includes the tweeks which are necessary to enable the
InterpreterOopMap.is_dead()
   function in the product mode as it was originally defined only under
#ifdef ENABLE_ZAP_DEAD_LOCALS.
   This makes the code in the oopMapCache.?pp a little bit more simple.


Testing:
   Running the failing tests: vm.mlvm.indy.func.jvmti
   In progress: nsk.jvmti, nsk.jdi, nsk.jdwp test runs on sparcv9 and amd64


Thanks,
Serguei



Re: RFR (S) 8013942: JSR 292: assert(type() == T_OBJECT) failed: type check

2014-06-26 Thread serguei.spit...@oracle.com

Thanks, Vladimir!
Serguei

On 6/26/14 3:02 PM, Vladimir Ivanov wrote:

Looks good.

Best regards,
Vladimir Ivanov

On 6/25/14 5:57 PM, serguei.spit...@oracle.com wrote:

Please, review the fix for:
   https://bugs.openjdk.java.net/browse/JDK-8013942


Open webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2014/hotspot/8013942-JVMTI-lobj.1 





Summary:

   This is a Nightly Stabilization issue.

   The problem is that the JVMTI GetLocalObject() function is hitting
the assert
   as the type of the local at current bci is not T_OBJECT as expected.
   It is because this local is alive only in a narrow scope of one
condition in the method but current bci is out of this csope.

   A fragment from the target method:

 static Class? canonicalize(Class? t, int how) {
 Class? ct; == The local
 if (t == Object.class) {
 // no change, ever
 } else if (!t.isPrimitive()) {
 switch (how) {
 case UNWRAP:
 ct = Wrapper.asPrimitiveType(t); == Initialized 
here

 if (ct != t) return ct;
 break;
 case RAW_RETURN:
 case ERASE:
 return Object.class;
 }
 } else if (t == void.class) {== The
GetLocalObject() is called with bci in this block
 // no change, usually
 switch (how) {
 case RAW_RETURN:
 return int.class;
 case WRAP:
 return Void.class;
 }
 } else {
 . . .

   The local 'ct' is only alive in the block of condition if
(!t.isPrimitive()).
   However, it is dead local in the next block.

   The fix suggested in the webrev above is to use the oop_mask for the
method's current bci.
   It tells when the local is dead in the scope of this bci.
   Return the JVMTI_ERROR_INVALID_SLOT error in such a case.

   The fix also includes the tweeks which are necessary to enable the
InterpreterOopMap.is_dead()
   function in the product mode as it was originally defined only under
#ifdef ENABLE_ZAP_DEAD_LOCALS.
   This makes the code in the oopMapCache.?pp a little bit more simple.


Testing:
   Running the failing tests: vm.mlvm.indy.func.jvmti
   In progress: nsk.jvmti, nsk.jdi, nsk.jdwp test runs on sparcv9 and 
amd64



Thanks,
Serguei