Oh, great, than no worries. // Katja
----- Original Message ----- From: staffan.lar...@oracle.com To: yekaterina.kantser...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Tuesday, July 1, 2014 5:20:57 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR: 8046883 com/sun/jdi/ProcessAttachTest.sh gets "java.io.IOException: Invalid process identifier" on windows Katja, I intentionally did not include javaoptions when launching the new JVMs since I could not think of any options that would affect how -agentlib:jdwp=suspend=y|n works or does not work (which is what this test verifies). /Staffan On 1 jul 2014, at 17:03, Yekaterina Kantserova < yekaterina.kantser...@oracle.com > wrote: Staffan, since I'm working on JDK-8048892 right now I happen to know test/com/sun/jdi/ProcessAttachTest.sh has used @debuggeeVMOptions. ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(...) will not add test.vm.options and test.java.options to the command line. You need to use: ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(Utils.addTestJavaOpts(...)) You can look at the ProcessTools.executeTestJvm() for more information. Thanks, Katja ----- Original Message ----- From: daniel.daughe...@oracle.com To: staffan.lar...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Tuesday, July 1, 2014 3:37:38 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR: 8046883 com/sun/jdi/ProcessAttachTest.sh gets "java.io.IOException: Invalid process identifier" on windows On 6/23/14 2:33 AM, Staffan Larsen wrote: Fancy! new review: http://cr.openjdk.java.net/~sla/8046883/webrev.01/ test/com/sun/jdi/ProcessAttachTest.java New test looks very good. There's a couple of really long lines, but I can't think of a way to break the lines that makes sense with this new streams coding style. Thumbs up. Dan /Staffan On 18 jun 2014, at 13:59, Peter Allwin < peter.all...@oracle.com > wrote: This looks a lot better! (Since we’re using fancy new features we could use streams to find the connector instance) AttachingConnector ac = Bootstrap.virtualMachineManager().attachingConnectors() .stream() .filter(c -> c.name().equals( "com.sun.jdi.ProcessAttach" )) .findFirst() .orElseThrow(() -> new RuntimeException( "Unable to locate ProcessAttachingConnector" )); Thanks! /peter On 17 Jun 2014, at 19:46, Staffan Larsen < staffan.lar...@oracle.com > wrote: Here is a rewrite of the test in Java instead of a shell script. Should be easier to maintain. webrev: http://cr.openjdk.java.net/~sla/8046883/webrev.00/ Thanks, /Staffan On 17 jun 2014, at 15:12, Staffan Larsen < staffan.lar...@oracle.com > wrote: On 17 jun 2014, at 15:03, Alan Bateman < alan.bate...@oracle.com > wrote: On 17/06/2014 13:35, Staffan Larsen wrote: : It could be a timing issue, but in the other direction. If cygwin hasn’t yet started the real windows process when I run ps, then maybe ps will not list it. But given the “sleep 2” before the ps invocation, the process should have had time to started. No guarantees of course. Making the sleep shorter will not help as the process we are starting will not terminate until we tell it to. Okay, although what I was suggesting is to use your patch but additionally move the sleep at L79 into the new while loop so that it doesn't spin quickly through the 10 iterations. That would give the test 10 attempts (and 10 seconds) to get the pid. Ah, I see. I misunderstood your comment. I started looking at rewriting the test in pure Java instead of the shell script. With the new Process.getPid() this looks like the best approach. I’ll come back with a new review request soon. /Staffan
