RFR 8205654: serviceability/dcmd/framework/HelpTest.java timed out

[serguei.spitsyn at oracle.com]

Daniil and David,

Just wanted to let you know that I'm reviewing this.


On 1/20/19 21:18, David Holmes wrote:
> Thanks for the update Daniil. I still remain concerned about the 
> robustness of the command-line parsing - this seems like a feature 
> that needs its own set of tests.


I guess, the main David's concern is new file
   src/jdk.jcmd/linux/classes/sun/tools/ProcessHelper.java

which includes introduced by Daniil command-line processing.

Thanks,
Serguei


>
> I'll leave it up to Serguei and others as to how to proceed.
>
> David
> -----
>
> On 19/01/2019 9:08 am, Daniil Titov wrote:
>> Hi David and Serguei,
>>
>> Please review a new version of the fix that now covers the case when 
>> Java executes a module with the main class name explicitly specified 
>> in the command line.
>>
>> Webrev: http://cr.openjdk.java.net/~dtitov/8205654/webrev.03
>> Bug: : https://bugs.openjdk.java.net/browse/JDK-8205654
>>
>> Thanks!
>> --Daniil
>>
>> On 1/8/19, 6:05 PM, "David Holmes" <david.holmes at oracle.com> wrote:
>>
>>      Hi Daniil,
>>           Sorry this slipped through the Xmas break cracks :)
>>           On 22/12/2018 12:04 pm, Daniil Titov wrote:
>>      > Hi David and Serguei,
>>      >
>>      > Please review a new version of the fix that for Linux platform 
>> uses the proc filesystem to retrieve the main class name for the 
>> running Java process.
>>      >
>>      > Webrev: http://cr.openjdk.java.net/~dtitov/8205654/webrev.02/
>>      > Bug: https://bugs.openjdk.java.net/browse/JDK-8205654
>>           It's more complex than I had envisaged but seems to be 
>> doing the job.
>>      I'm not sure how robust the command-line parsing is, in 
>> particular it
>>      doesn't handle these forms:
>>               or  java [options] -m <module>[/<mainclass>] [args...]
>>              java [options] --module <module>[/<mainclass>] [args...]
>>                  (to execute the main class in a module)
>>           I can't really comment on all the details.
>>           Thanks,
>>      David
>>      -----
>>           > Thanks,
>>      > Daniil
>>      >
>>      > On 11/29/18, 4:52 PM, "David Holmes" 
>> <david.holmes at oracle.com> wrote:
>>      >
>>      >      Hi Daniil,
>>      >
>>      >      On 30/11/2018 7:30 am, Daniil Titov wrote:
>>      >      > Thank you, David!
>>      >      >
>>      >      > The proposed fix didn't help. It still hangs at some 
>> occasions.  Additional tracing showed that when jcmd is invoked with 
>> the main class name it iterates over all running Java processes and 
>> temporary attaches to them to retrieve the main class name. It hangs 
>> while trying to attach to one of the running Java processes. There 
>> are numerous Java processes running at the host machine some 
>> associated with the test framework itself and another with the tests 
>> running in parallel. It is not clear what exact is this particular 
>> process since the jcmd hangs before retrieving the process' main 
>> class name, but after all tests terminated the process with this id 
>> is no longer running.  I have to revoke this review since more 
>> investigation is required.
>>      >
>>      >      That sounds like an unsolvable problem for the test. You 
>> can't control
>>      >      other Java processes on the machine, and searching by 
>> name requires
>>      >      asking each of them in turn.
>>      >
>>      >      How do we get the list of Java processes in the first 
>> place? Perhaps we
>>      >      need to do some /proc/<pid>/cmdline peeking?
>>      >
>>      >      Cheers,
>>      >      David
>>      >
>>      >      >
>>      >      > Best regards,
>>      >      > Daniil
>>      >      >
>>      >      >
>>      >      >
>>      >      > On 11/11/18, 1:35 PM, "David Holmes" 
>> <david.holmes at oracle.com> wrote:
>>      >      >
>>      >      >      Hi Daniil,
>>      >      >
>>      >      >      I took a quick look at this one ... two minor 
>> comments
>>      >      >
>>      >      >      The static class names could just be "Process" as 
>> they will acquire the
>>      >      >      enclosing class name as part of their own name 
>> anyway. As it is this
>>      >      >      gets repeated eg:
>>      >      >
>>      >      >      HelpTest$HelpTestProcess
>>      >      > InvalidCommandTest$InvalidCommandTestProcess
>>      >      >
>>      >      >      TestJavaProcess.java:
>>      >      >
>>      >      >      39     public static void main(String argv[]) {
>>      >      >
>>      >      >      Nit: Should be "String[] argv" in Java style
>>      >      >
>>      >      >      Thanks,
>>      >      >      David
>>      >      >
>>      >      >      On 10/11/2018 3:18 PM, Daniil Titov wrote:
>>      >      >      > Please review the change that fixes 
>> serviceability/dcmd/framework/* tests from a time out. The fix for 
>> JDK-8166642 made serviceability/dcmd/framework/* tests non-concurrent 
>> to ensure that they don't interact with each other and there are no 
>> multiple tests running simultaneously since all they do share the 
>> common main class name com.sun.javatest.regtest.agent.MainWrapper. 
>> However, it looks like the  tests from other directories still might 
>> run in parallel with these tests and they also have 
>> com.sun.javatest.regtest.agent.MainWrapper as a main class.
>>      >      >      >
>>      >      >      > The fix  ensures that each 
>> serviceability/dcmd/framework/* test uses a Java process with a 
>> unique main class name when connecting to this process with jcmd and 
>> the main class name.
>>      >      >      >
>>      >      >      > Bug: 
>> https://bugs.openjdk.java.net/browse/JDK-8205654
>>      >      >      > Webrev: 
>> http://cr.openjdk.java.net/~dtitov/8205654/webrev.001/
>>      >      >      >
>>      >      >      > Best regards,
>>      >      >      > Daniil
>>      >      >      >
>>      >      >      >
>>      >      >
>>      >      >
>>      >      >
>>      >
>>      >
>>      >
>>
>>