RFR 8024055: serviceability/attach/AttachWithStalePidFile.java createJavaPidFile() fails

[Jaroslav Bachorik]

Hi Serguei,

On 24.3.2015 10:22, serguei.spitsyn at oracle.com wrote:
> 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?

No, NULL will be returned only in case of EOF (eg. stream has been 
closed). Otherwise the readLine() will block until some data is available.

>   The while loop will complete if line == null.
>
>   Do we really want this condition? :
>     104     while (line == null || !line.equals(AttachWithStalePidFileTarget.READY_MSG)) {

The crucial part is the comparison with the READY_MSG which indicates 
that the target VM has actually passed the initialization and is ready 
for the test. The 'line != null' is necessary to prevent NPE being 
thrown. While the test would fail anyway it is more informational to 
fail while trying to attach than an NPE.

-JB-

>
>
> 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.bachorik at 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-
>>>
>>
>