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" <[email protected]> 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" <[email protected]> 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
     >      >
     >      >
     >
     >
     >

Reply via email to