RFR: JDK-8218754: JDK-8068225 regression in JDIBreakpointTest

[gary.adams at oracle.com]

Successfully ran 100X {solaris,macosx,windows,linux} debug builds
for the tests impacted by Debugee.endDebugee() processing.

|test/hotspot/jtreg/vmTestbase/nsk/jdi 
test/hotspot/jtreg/vmTestbase/vm/mlvm test/jdk/com/sun/jdi|

On 2/12/19 7:40 AM, Gary Adams wrote:
> On 2/12/19, 7:14 AM, David Holmes wrote:
>> On 12/02/2019 10:11 pm, Gary Adams wrote:
>>> Yes, see the revised webrev, we do have to guard against
>>> multiple calls to dispose, e.g. catch and ignore VMDisconnectException.
>>> We don't need to guard against a null vm. That would only exist if
>>> the vm was never initialized and we were shutting down.
>>
>> Okay. Revised webrev is better in that regard.
> One more round to add the import for VMDisconnectedException.
>
>   Webrev: http://cr.openjdk.java.net/~gadams/8218754/webrev.01/
>>
>>> There is a race condition between the debuggee and the debugger 
>>> process.
>>> In the original endDebugee, it attempted to do 2 things in parallel.
>>> By calling dispose then waitFor, in most cases the debugee would finish
>>> and report status before all the dispose operations completed.
>>> But some tests would fail if the dispose happened quicker than the 
>>> debuggee
>>> could report final exit status.
>>>
>>> The tests based on JDIBreakpointTest on  the other hand don't really
>>> care about the the final exit status. Once all the events have been 
>>> seen and handled, the test wants to shutdown the session.
>>
>> I'm still not seeing how JDIBreakpointTest started hanging/timing-out 
>> after you switched the order of dispose and waitFor. Does dispose 
>> affect the debuggee process or the debugger process?
> The JDIBreakpointTest would timeout on the endDebugee call to waitFor.
> There was no reason for the debugee to exit at that point.
> The call to dispose, provided the debugee with it's reason to exit.
> Not sure which operation induces the debugee to exit.
>>
>> Thanks,
>> David
>>
>>>
>>> On 2/12/19, 6:59 AM, David Holmes wrote:
>>>> Hi Gary,
>>>>
>>>> On 12/02/2019 8:08 pm, gary.adams at oracle.com wrote:
>>>>> The recent change to JDK-8068225 changed the order of operations
>>>>> in Debugee.endDebugee() to wait for the debugee to exit before
>>>>> disposing of the vm on the debugger side of the connection.
>>>>> For the tests based on JDIBreakpointTest the debuggee exit
>>>>> status is not used and the tests relied on the
>>>>> debugger side dispose operation to end the test.
>>>>>
>>>>> Since JDIBreakpointTest already includes a call to wait for
>>>>> the debugee, if does not need to use endDebuggee()
>>>>> to dispose and wait for the debugee to finish.
>>>>
>>>> I agree that potentially calling waitFor twice seems pointless. But 
>>>> how did the reordering of vm.dispose() and waitFor() cause all 
>>>> these tests to hang if they were waiting anyway? Does vm.dispose() 
>>>> have an effect on destroying the process?
>>>>
>>>> Also what concerns me is that dispose() is not resilient the way 
>>>> that endDebuggee is:
>>>>
>>>>  public void dispose() {
>>>>     vm.dispose();
>>>>  }
>>>>
>>>> versus
>>>>
>>>>     public int endDebugee() {
>>>>         if (vm != null) {
>>>>             try {
>>>>                 vm.dispose();
>>>>             } catch (VMDisconnectedException ignore) {
>>>>             }
>>>>             vm = null;
>>>>         }
>>>>
>>>> Do we need to be concerned with a null VM or getting 
>>>> VMDisconnectedException?
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> Testing in progress. The vm/mlvm tests are included in tiers 2, 3 
>>>>> and 6.
>>>>>
>>>>> diff --git 
>>>>> a/test/hotspot/jtreg/vmTestbase/vm/mlvm/share/jdi/JDIBreakpointTest.java 
>>>>> b/test/hotspot/jtreg/vmTestbase/vm/mlvm/share/jdi/JDIBreakpointTest.java 
>>>>>
>>>>> --- 
>>>>> a/test/hotspot/jtreg/vmTestbase/vm/mlvm/share/jdi/JDIBreakpointTest.java 
>>>>>
>>>>> +++ 
>>>>> b/test/hotspot/jtreg/vmTestbase/vm/mlvm/share/jdi/JDIBreakpointTest.java 
>>>>>
>>>>> @@ -1,5 +1,5 @@
>>>>>   /*
>>>>> - * Copyright (c) 2011, 2018, Oracle and/or its affiliates. All 
>>>>> rights reserved.
>>>>> + * Copyright (c) 2011, 2019, Oracle and/or its affiliates. All 
>>>>> rights reserved.
>>>>>    * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>>>>    *
>>>>>    * This code is free software; you can redistribute it and/or 
>>>>> modify it
>>>>> @@ -359,7 +359,7 @@
>>>>>           }.go();
>>>>>
>>>>>           if (!debuggee.terminated())
>>>>> -            debuggee.endDebugee();
>>>>> +            debuggee.dispose();
>>>>>
>>>>>           debuggee.waitFor();
>>>>>           return true;
>>>>>
>>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190212/18c6c6a1/attachment-0001.html>