Hi

The idea basically looks good. I think it just make a sense to polish it a 
little bit to hide "sh" usage from test and get core from OutputAnalyzer.
 
Also, there is a 'CrashApp' in ClhsdbCDSCore.java. Makes it sense to unify it 
with LingeredApp crasher? Currently, it uses Unsafe to crash application.

Also, crashes are used in other tests, I see some implementations in 
open/test/hotspot/jtreg/vmTestbase/vm/share/vmcrasher

So it would be nice to have some common way to crash hotspot.

Leonid


> On Jun 25, 2020, at 2:41 PM, Chris Plummer <[email protected]> wrote:
> 
> Hello,
> 
> Please help with a preliminary review of changes to add better support for 
> writing SA tests that work on core files:
> 
> https://bugs.openjdk.java.net/browse/JDK-8248194
> http://cr.openjdk.java.net/~cjplummer/8248194/webrev.00/index.html
> 
> As pointed out, this is a preliminary review. I suspect there will be some 
> feedback for changes/improvements. Also, I still need to work out a final 
> solution for how to get LingeredApp to produce a crash. What I currently have 
> works but is somewhat of a hack w.r.t. the makefile change, so you can ignore 
> the makefiile change for now. I'm working on a more proper solution with the 
> build team.
> 
> As outlined in the CR, these are the 3 main goals of this CR:
> 
> 1. SATestUtils should include support for finding the core file. This 
> includes parsing the output of the crashed process to locate where the core 
> file was saved, and returning this location to the user.
> 
> 2. SATestUtils should include support for adding the "ulimit -c unlimited" 
> prefix to the command that will produce the core file, allowing the 
> overriding of any lower limit so we can be sure the core file will be 
> produced.
> 
> 3. LingeredApp should include support for producing a core file.
> 
> As proof of concept for these 3 changes in test library support, I'm updating 
> the following 3 tests:
> 
> ClhsdbCDSCore.java: Use the SATestUtils support listed above. This test does 
> not use LingeredApp, so those improvements don't apply.
> 
> TestJmapCore.java: Use the SATestUtils support listed above. This test does 
> not use LingeredApp, so those improvements don't apply.
> 
> ClhsdbFindPC.java: Use all the above features, including having LingeredApp 
> produce a core file. This is the only test modified to start testing on core 
> files that didn't previously do so. It still also tests on a live process.
> 
> In the future more Clhsdb tests will be converted to work on core files in a 
> manner similar to ClhsdbFindPC.
> 
> The new SATestUtils code is borrowed from (more like ripped out of) 
> ClhsdbCDSCore.java and TestJmapCore.java. They both had a lot of code 
> dedicated to finding the core file and also applying "ulimit -c unlimitted" 
> if necessary, but didn't do so in quite the same way. Now both these tests 
> share code in SATestUtils.java. One thing I did drop is TestJmapCore.java use 
> of ":KILLED_PID" in the output to help find the core file. It's no longer 
> necessary based on the smarter core locating code I pulled from 
> ClhsdbCDSCore.java.
> 
> thanks,
> 
> Chris

Reply via email to