[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR: 8046883 com/sun/jdi/ProcessAttachTest.sh gets "java.io.IOException: Invalid process identif
From: Yekaterina Kantserova <yekaterina.kantserova () oracle ! com>
Date: 2014-07-01 15:29:36
Message-ID: 2b7bc04b-5fe7-442d-ba78-d951372c98f1 () default
[Download RAW message or body]
Oh, great, than no worries.
// Katja
----- Original Message -----
From: staffan.larsen@oracle.com
To: yekaterina.kantserova@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.kantserova@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.daugherty@oracle.com
To: staffan.larsen@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.allwin@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.larsen@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.larsen@oracle.com > wrote:
On 17 jun 2014, at 15:03, Alan Bateman < Alan.Bateman@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
[Attachment #3 (text/html)]
<html><head><style type='text/css'>p { margin: 0; }</style></head><body><div \
style='font-family: Times New Roman; font-size: 12pt; color: #000000'>Oh, great, than \
no worries.<br><br>// Katja<br><br>----- Original Message -----<br>From: \
staffan.larsen@oracle.com<br>To: yekaterina.kantserova@oracle.com<br>Cc: \
serviceability-dev@openjdk.java.net<br>Sent: Tuesday, July 1, 2014 5:20:57 PM GMT \
+01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna<br>Subject: Re: RFR: \
8046883 com/sun/jdi/ProcessAttachTest.sh gets "java.io.IOException: Invalid process \
identifier" on windows<br><br><div style="word-wrap: break-word; -webkit-nbsp-mode: \
space; -webkit-line-break: after-white-space;">Katja,<div><br></div><div>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).</div><div><br></div><div>/Staffan</div><div><br></div><div><br><div><div>On \
1 jul 2014, at 17:03, Yekaterina Kantserova <<a \
href="mailto:yekaterina.kantserova@oracle.com" \
target="_blank">yekaterina.kantserova@oracle.com</a>> wrote:</div><br \
class="Apple-interchange-newline"><blockquote><div style="font-family: Helvetica; \
font-size: 16px; font-style: normal; font-variant: normal; font-weight: normal; \
letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; \
text-indent: 0px; text-transform: none; white-space: normal; widows: auto; \
word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div style="font-family: 'Times \
New Roman'; font-size: 12pt;">Staffan,<br><br>since I'm working on JDK-8048892 right \
now I happen to know test/com/sun/jdi/ProcessAttachTest.sh has used \
@debuggeeVMOptions.<br><br><font face="Courier New, courier, monaco, monospace, \
sans-serif">ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(...)<span \
class="Apple-converted-space"> </span></font><br><br>will not add \
test.vm.options and test.java.options to the command line. You need to \
use:<br><br><font face="Courier New, courier, monaco, monospace, \
sans-serif">ProcessBuilder pb = \
ProcessTools.createJavaProcessBuilder(Utils.addTestJavaOpts(...))<span \
class="Apple-converted-space"> </span></font><br><br>You can look at the \
ProcessTools.executeTestJvm() for more \
information.<br><br>Thanks,<br>Katja<br><br><br><br>----- Original Message \
-----<br>From:<span class="Apple-converted-space"> </span><a \
href="mailto:daniel.daugherty@oracle.com" \
target="_blank">daniel.daugherty@oracle.com</a><br>To:<span \
class="Apple-converted-space"> </span><a href="mailto:staffan.larsen@oracle.com" \
target="_blank">staffan.larsen@oracle.com</a><br>Cc:<span \
class="Apple-converted-space"> </span><a \
href="mailto:serviceability-dev@openjdk.java.net" \
target="_blank">serviceability-dev@openjdk.java.net</a><br>Sent: Tuesday, July 1, \
2014 3:37:38 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / \
Vienna<br>Subject: Re: RFR: 8046883 com/sun/jdi/ProcessAttachTest.sh gets \
"java.io.IOException: Invalid process identifier" on windows<br><br><div>On 6/23/14 \
2:33 AM, Staffan Larsen wrote:<br><blockquote \
cite="mid:AAAD2855-6911-41C8-97AE-FC51C4220CC4@oracle.com">Fancy!<div><br></div><div>new \
review:<span class="Apple-converted-space"> </span><a \
href="http://cr.openjdk.java.net/%7Esla/8046883/webrev.01/" \
target="_blank">http://cr.openjdk.java.net/~sla/8046883/webrev.01/</a></div></blockquote><tt> <br>test/com/sun/jdi/ProcessAttachTest.java<br> \
New test looks very good. There's a couple of really long \
lines,<br> but I can't think of a way to break the lines that makes \
sense<br> with this new streams coding style.<br><br>Thumbs \
up.<br> <br>Dan<br><br><br></tt><blockquote \
cite="mid:AAAD2855-6911-41C8-97AE-FC51C4220CC4@oracle.com"><div><br></div><div>/Staffan</div><div><br><div><div>On \
18 jun 2014, at 13:59, Peter Allwin <<a href="mailto:peter.allwin@oracle.com" \
target="_blank">peter.allwin@oracle.com</a>> wrote:</div><br \
class="Apple-interchange-newline"><blockquote><div style="word-wrap: break-word; \
-webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">This looks a lot \
better!<div><br></div><div>(Since we're using fancy new features we could use streams \
to find the connector instance)</div><div><br></div><div><div style="margin: 0px; \
font-size: 11px; font-family: Menlo;"> \
AttachingConnector ac = \
Bootstrap.virtualMachineManager().attachingConnectors()</div><div style="margin: 0px; \
font-size: 11px; font-family: Menlo;"> \
.stream()</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"> \
.filter(c -> c.name().equals(<span \
style="color: rgb(209, 47, 27);">"com.sun.jdi.ProcessAttach"</span>))</div><div \
style="margin: 0px; font-size: 11px; font-family: Menlo;"> \
.findFirst()</div><div style="margin: 0px; font-size: 11px; \
font-family: Menlo;"> .orElseThrow(() \
-><span class="Apple-converted-space"> </span><span style="color: rgb(187, \
44, 162);">new</span><span \
class="Apple-converted-space"> </span>RuntimeException(<span style="color: \
rgb(209, 47, 27);">"Unable to locate \
ProcessAttachingConnector"</span>));</div><div><br></div><div>Thanks!</div><div>/peter</div><div><br></div><div><div>On \
17 Jun 2014, at 19:46, Staffan Larsen <<a href="mailto:staffan.larsen@oracle.com" \
target="_blank">staffan.larsen@oracle.com</a>> wrote:</div><br \
class="Apple-interchange-newline"><blockquote><div style="word-wrap: break-word; \
-webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Here is a rewrite \
of the test in Java instead of a shell script. Should be easier to \
maintain.<div><br></div><div>webrev:<span \
class="Apple-converted-space"> </span><a \
href="http://cr.openjdk.java.net/%7Esla/8046883/webrev.00/" \
target="_blank">http://cr.openjdk.java.net/~sla/8046883/webrev.00/</a></div><div><br></div><div><div>Thanks,</div><div>/Staffan</div></div><div><br><div><div>On \
17 jun 2014, at 15:12, Staffan Larsen <<a href="mailto:staffan.larsen@oracle.com" \
target="_blank">staffan.larsen@oracle.com</a>> wrote:</div><br \
class="Apple-interchange-newline"><blockquote><div style="font-size: 16px; \
font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: \
normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; \
text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; \
-webkit-text-stroke-width: 0px;"><br>On 17 jun 2014, at 15:03, Alan Bateman <<a \
href="mailto:Alan.Bateman@oracle.com" target="_blank">Alan.Bateman@oracle.com</a>> \
wrote:<br><br><blockquote>On 17/06/2014 13:35, Staffan Larsen \
wrote:<br><blockquote>:<br><br>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.<br><br>Making the \
sleep shorter will not help as the process we are starting will not terminate until \
we tell it to.<br><br><br></blockquote>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.<br></blockquote><br>Ah, I see. I misunderstood your \
comment.<br><br>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.<br><br>/Staffan</div></blockquote></div></div></div></blockquote></div></div></d \
iv></blockquote></div></div></blockquote></div></div></div></blockquote></div><br></div></div></div></body></html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic