Re: RFR JDK-8031554: com/sun/tools/attach/BasicTests.java fails intermittently
On 07/01/2014 11:40 PM, shanliang wrote: Jaroslav Bachorik wrote: Hi Shanliang, On 07/01/2014 09:47 PM, shanliang wrote: I am still thinking to keep the original fix, because: 1) to throw InterruptedException does not fix the test failure, it might give more info for diagnostics. If it was really caused by an InterruptedException, then to fix the issue we still need to know who could interrupt the test main thread, in which case and why, and whether possible to ignore it (skip the test or try again?). I'm sorry but I can't agree with this. The proposed patch does not add anything else than making the InterruptedException visible. Adding process.waitFor() will solve nothing as it is already called by ProcessTools.getOutput(process) while creating OutputAnalyzer. 2) the test library is used also by other tests and to modify it might make new fail, it is better to concentrate at first on a single test before knowing the exact cause. I wouldn't expect new failures - when an InterruptedException was thrown the ProcessTools.executeTestJvm(args) returned null. Either the test would fail with NPE or custom assert - it makes no sense to work with null process. IMO, this should be fixed in the test library. Sorry I may miss something here, you suggested: 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. We still need to do something when an InterruptedException happens, skip the test or retry? before making a decision we should know why there was an InterruptedException and in which case, I really do not have any idea, and I do not want to exclude other possibilities. The most probable explanation, based on the analysis of many intermittent test failures, is that the harness is trying to time out the test. But it is just an educated guess ... Yes what my fix does is to expose an InterruptedException if it happens, but it could make sure that it was really because of an InterruptedException. I don't feel particularly happy about the code duplication introduced by this fix. But if official reviewers are fine with it I won't block this review. About new failure, there could be a negative test which expected a IllegalStateException -- failed OutputAnalyser, who knows. My concern is that there also might be other tests failing intermittently (or providing wrong results) due to the InterruptedException being silently swallowed. You might run the whole testsuite with the modified ProcessTools.getOutput() on JPRT and see if there are any negative tests failing (they should since instead of null value they will receive InterryptedException). -JB- Shanliang -JB- Shanliang Christian Tornqvist wrote: I can’t remember if there was a reason for doing it like this, but it seems reasonable to not catch the InterruptedException in getOutput(). Thanks, Christian *From:*Staffan Larsen [mailto:staffan.lar...@oracle.com] *Sent:* Friday, June 27, 2014 4:49 AM *To:* shanliang *Cc:* Jaroslav Bachorik; serviceability-dev@openjdk.java.net serviceability-dev@openjdk.java.net; Christian Tornqvist *Subject:* Re: RFR JDK-8031554: com/sun/tools/attach/BasicTests.java fails intermittently It does look suspicious to catch and ignore the InterruptedException, especially since the OutputAnalyzer constructor will fail in this case. Christian is the original author of these classes: do you know if there is a good rationale for doing this? Or should we propagate the InterruptedException? Thanks, /Staffan On 26 jun 2014, at 17:24, shanliang shanliang.ji...@oracle.com mailto:shanliang.ji...@oracle.com wrote: 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
Re: RFR JDK-8031554: com/sun/tools/attach/BasicTests.java fails intermittently
Jaroslav Bachorik wrote: On 07/01/2014 11:40 PM, shanliang wrote: Jaroslav Bachorik wrote: Hi Shanliang, On 07/01/2014 09:47 PM, shanliang wrote: I am still thinking to keep the original fix, because: 1) to throw InterruptedException does not fix the test failure, it might give more info for diagnostics. If it was really caused by an InterruptedException, then to fix the issue we still need to know who could interrupt the test main thread, in which case and why, and whether possible to ignore it (skip the test or try again?). I'm sorry but I can't agree with this. The proposed patch does not add anything else than making the InterruptedException visible. Adding process.waitFor() will solve nothing as it is already called by ProcessTools.getOutput(process) while creating OutputAnalyzer. 2) the test library is used also by other tests and to modify it might make new fail, it is better to concentrate at first on a single test before knowing the exact cause. I wouldn't expect new failures - when an InterruptedException was thrown the ProcessTools.executeTestJvm(args) returned null. Either the test would fail with NPE or custom assert - it makes no sense to work with null process. IMO, this should be fixed in the test library. Sorry I may miss something here, you suggested: 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. We still need to do something when an InterruptedException happens, skip the test or retry? before making a decision we should know why there was an InterruptedException and in which case, I really do not have any idea, and I do not want to exclude other possibilities. The most probable explanation, based on the analysis of many intermittent test failures, is that the harness is trying to time out the test. But it is just an educated guess ... Thought about this possibility, but a harness would kill the test instead to interrupt the test thread? Yes what my fix does is to expose an InterruptedException if it happens, but it could make sure that it was really because of an InterruptedException. I don't feel particularly happy about the code duplication introduced by this fix. But if official reviewers are fine with it I won't block this review. About new failure, there could be a negative test which expected a IllegalStateException -- failed OutputAnalyser, who knows. My concern is that there also might be other tests failing intermittently (or providing wrong results) due to the InterruptedException being silently swallowed. I agree that we need to do investigation on the test library, but better to do it later when we have more info from this test. Thanks, Shanliang You might run the whole testsuite with the modified ProcessTools.getOutput() on JPRT and see if there are any negative tests failing (they should since instead of null value they will receive InterryptedException). -JB- Shanliang -JB- Shanliang Christian Tornqvist wrote: I can’t remember if there was a reason for doing it like this, but it seems reasonable to not catch the InterruptedException in getOutput(). Thanks, Christian *From:*Staffan Larsen [mailto:staffan.lar...@oracle.com] *Sent:* Friday, June 27, 2014 4:49 AM *To:* shanliang *Cc:* Jaroslav Bachorik; serviceability-dev@openjdk.java.net serviceability-dev@openjdk.java.net; Christian Tornqvist *Subject:* Re: RFR JDK-8031554: com/sun/tools/attach/BasicTests.java fails intermittently It does look suspicious to catch and ignore the InterruptedException, especially since the OutputAnalyzer constructor will fail in this case. Christian is the original author of these classes: do you know if there is a good rationale for doing this? Or should we propagate the InterruptedException? Thanks, /Staffan On 26 jun 2014, at 17:24, shanliang shanliang.ji...@oracle.com mailto:shanliang.ji...@oracle.com wrote: 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
Re: RFR(M): 8028474: sun/jvmstat/monitor/MonitoredVm/MonitorVmStartTerminate.sh timeout, leaves looping process behind
On 1/07/2014 10:35 PM, roger riggs wrote: Hi Erik, Consider switching to System.nanoTime; it is not sensitive to clock changes Not completely true unfortunately. The change to use CLOCK_MONOTONIC_RAW will make it true (OS bugs not withstanding). But I would suggest switching anyway. David and avoids leaving a land mine that may cause a spurious non-repeatable test failure. 'Deducing it from the log' means there is a failure and creates probably an hour or two of work for some quality engineer and burns a couple of hours re-running the test run. Roger On 7/1/2014 3:37 AM, Erik Gahlin wrote: JavaProcess.waitForRemoval: How about using timestamps (currentTimeMillis()) before the loop and for each ite ration to determine if the timeout has expired (instead of time+=100”)? The code now uses currentTimeMillis(). Premature timeouts due to clock changes can be deduced from the log.
Re: RFR JDK-8031554: com/sun/tools/attach/BasicTests.java fails intermittently
I agree with Jaroslav here, and would like a different patch. The proposed patch does not solve the problem and as a debugging help it is implemented at the wrong level. I think the ProcessTools.getOutput() should be updated to throw the InterruptedException - this looks wrong in the library. The impact on other tests should be minimal, I don’t believe anyone depends on getOutput() swallowing InterruptedException - if they do, we have to fix that, too (which at the same time would make those tests more stable). Thanks, /Staffan On 2 jul 2014, at 10:05, shanliang shanliang.ji...@oracle.com wrote: Jaroslav Bachorik wrote: On 07/01/2014 11:40 PM, shanliang wrote: Jaroslav Bachorik wrote: Hi Shanliang, On 07/01/2014 09:47 PM, shanliang wrote: I am still thinking to keep the original fix, because: 1) to throw InterruptedException does not fix the test failure, it might give more info for diagnostics. If it was really caused by an InterruptedException, then to fix the issue we still need to know who could interrupt the test main thread, in which case and why, and whether possible to ignore it (skip the test or try again?). I'm sorry but I can't agree with this. The proposed patch does not add anything else than making the InterruptedException visible. Adding process.waitFor() will solve nothing as it is already called by ProcessTools.getOutput(process) while creating OutputAnalyzer. 2) the test library is used also by other tests and to modify it might make new fail, it is better to concentrate at first on a single test before knowing the exact cause. I wouldn't expect new failures - when an InterruptedException was thrown the ProcessTools.executeTestJvm(args) returned null. Either the test would fail with NPE or custom assert - it makes no sense to work with null process. IMO, this should be fixed in the test library. Sorry I may miss something here, you suggested: 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. We still need to do something when an InterruptedException happens, skip the test or retry? before making a decision we should know why there was an InterruptedException and in which case, I really do not have any idea, and I do not want to exclude other possibilities. The most probable explanation, based on the analysis of many intermittent test failures, is that the harness is trying to time out the test. But it is just an educated guess ... Thought about this possibility, but a harness would kill the test instead to interrupt the test thread? Yes what my fix does is to expose an InterruptedException if it happens, but it could make sure that it was really because of an InterruptedException. I don't feel particularly happy about the code duplication introduced by this fix. But if official reviewers are fine with it I won't block this review. About new failure, there could be a negative test which expected a IllegalStateException -- failed OutputAnalyser, who knows. My concern is that there also might be other tests failing intermittently (or providing wrong results) due to the InterruptedException being silently swallowed. I agree that we need to do investigation on the test library, but better to do it later when we have more info from this test. Thanks, Shanliang You might run the whole testsuite with the modified ProcessTools.getOutput() on JPRT and see if there are any negative tests failing (they should since instead of null value they will receive InterryptedException). -JB- Shanliang -JB- Shanliang Christian Tornqvist wrote: I can’t remember if there was a reason for doing it like this, but it seems reasonable to not catch the InterruptedException in getOutput(). Thanks, Christian *From:*Staffan Larsen [mailto:staffan.lar...@oracle.com] *Sent:* Friday, June 27, 2014 4:49 AM *To:* shanliang *Cc:* Jaroslav Bachorik; serviceability-dev@openjdk.java.net serviceability-dev@openjdk.java.net; Christian Tornqvist *Subject:* Re: RFR JDK-8031554: com/sun/tools/attach/BasicTests.java fails intermittently It does look suspicious to catch and ignore the InterruptedException, especially since the OutputAnalyzer constructor will fail in this case. Christian is the original author of these classes: do you know if there is a good rationale for doing this? Or should we propagate the InterruptedException? Thanks, /Staffan On 26 jun 2014, at 17:24, shanliang shanliang.ji...@oracle.com mailto:shanliang.ji...@oracle.com wrote: 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
Re: Fwd: PowerPC issue: Some JVMTI dynamic code generated events have code size of zero
Adding the Serviceability team to the thread since JVM/TI is owned by them... Dan On 7/2/14 10:15 AM, Maynard Johnson wrote: Cross-posting to see if Hotspot developers can help. -Maynard Original Message Subject: PowerPC issue: Some JVMTI dynamic code generated events have code size of zero Date: Wed, 25 Jun 2014 10:18:17 -0500 From: Maynard Johnson mayna...@us.ibm.com To: ppc-aix-port-...@openjdk.java.net ppc-aix-port-...@openjdk.java.net Hello, PowerPC OpenJDK folks, I am just now starting to get involved in the OpenJDK project. My goal is to ensure that the standard serviceability tools and tooling (jdb, JVMTI, jmap, etc.) work correctly on the PowerLinux platform. I selected JVMTI to start with since I have some experience from a client perspective with the JVMTI API. An OSS profiling tool for which I am the maintainer (oprofile) provides an agent library that implements the JVMTI API. Using this agent library to profile Java apps on my Intel-based laptop with OpenJDK (using various versions, up to current jdk9-dev) works fine. But the same profiling scenario attempted on my PowerLinux box (POWER7/Fedora 20) fails miserably. The oprofile agent library registers for callbacks for CompiledMethodLoad, CompiledMethodUnload, and DynamicCodeGenerated. In the callback functions, it writes information about the JVMTI event to a file. After profiling completes, oprofile's post-processing phase involves interpreting the information from the agent library's output file and generating an ELF file to represent the JITed code. When I profile an OpenJDK app on my Power system, the post-processing phase fails while trying to resolve overlapping symbols. The failure is due to the fact that it is unexpectedly finding symbols with code size of zero overlapping at the starting address of some other symbol with non-zero code size. The symbols in question here are from DynamicCodeGenerated events. Are these code size=0 events valid? If so, I can fix the oprofile code to handle them. If they're not valid, then below is some debug information I've collected so far. I instrumented JvmtiExport::post_dynamic_code_generated_internal (in hotspot/src/share/vm/prims/jvmtiExport.cpp) to print a debug line when a symbol with code size of zero was detected and then ran the following command: java -agentpath:jdk9-install-dir/jvm/openjdk-1.9.0-internal/demo/jvmti/CodeLoadInfo/lib/libCodeLoadInfo.so -version The debug output from my instrumentation was as follows: Code size is ZERO!! Dynamic code generated event sent for flush_icache_stub; code begin: 0x3fff6880; code end: 0x3fff6880 Code size is ZERO!! Dynamic code generated event sent for throw_exception; code begin: 0x3fff68000a90; code end: 0x3fff68000a90 Code size is ZERO!! Dynamic code generated event sent for throw_exception; code begin: 0x3fff68016600; code end: 0x3fff68016600 Code size is ZERO!! Dynamic code generated event sent for throw_exception; code begin: 0x3fff68016600; code end: 0x3fff68016600 Code size is ZERO!! Dynamic code generated event sent for throw_exception; code begin: 0x3fff68016600; code end: 0x3fff68016600 Code size is ZERO!! Dynamic code generated event sent for verify_oop; code begin: 0x3fff6801665c; code end: 0x3fff6801665c openjdk version 1.9.0-internal OpenJDK Runtime Environment (build 1.9.0-internal-mpj_2014_06_18_09_55-b00) OpenJDK 64-Bit Server VM (build 1.9.0-internal-mpj_2014_06_18_09_55-b00, mixed mode) I don't have access to an AIX system to know if the same issue would be seen there. Let me know if there's any other information I can provide. Thanks for the help. -Maynard
