Jaroslav,
test/serviceability/attach/AttachWithStalePidFile.java
A question:
101 private static void waitForTargetReady(Process target) throws
IOException {
102 BufferedReader br = new BufferedReader(new
InputStreamReader(target.getInputStream()));
103 String line = br.readLine();
104 while (line != null &&
!line.equals(AttachWithStalePidFileTarget.READY_MSG)) {
105 line = br.readLine();
106 }
107 // target VM ready
108 }
Not sure, in what condition the br.readLine() returns null.
Will null be returned if the target thread did not write anything to the
stdout yet?
The while loop will complete if line == null.
Do we really want this condition? :
104 while (line == null ||
!line.equals(AttachWithStalePidFileTarget.READY_MSG)) {
Otherwise, it looks good.
Thanks,
Serguei
On 3/23/15 12:20 PM, Jaroslav Bachorik wrote:
On 23.3.2015 13:44, Staffan Larsen wrote:
Looks good, but please print the exception at line 118 in
AttachWithStalePidFile.java.
Hm, like this http://cr.openjdk.java.net/~jbachorik/8024055/webrev.01 ?
-JB-
Thanks,
/Staffan
On 23 mar 2015, at 12:42, Jaroslav Bachorik
<jaroslav.bacho...@oracle.com> wrote:
Please, review the following test change
Issue : https://bugs.openjdk.java.net/browse/JDK-8024055
Webrev: http://cr.openjdk.java.net/~jbachorik/8024055/webrev.00
This request is a follow-up to the stalled review request
http://mail.openjdk.java.net/pipermail/serviceability-dev/2014-October/015785.html
(the issue has changed its owner since then)
As stated in the original request:
"
This patch fixes two intermittent issues seen over the past year:
a) Possible failure where an existing pid-file is not owned by the
test user
b) Race during startup where we try to attach to the target before
it’s ready (removed arbitrary 5sec sleep)
"
This version is addressing David's comment about better processing
the target process' stdout directly and not asynchronously.
Thanks,
-JB-
