RFR: JDK-8218754: JDK-8068225 regression in JDIBreakpointTest
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Tue Feb 12 20:10:42 UTC 2019
Hi Gary,
+1
Thanks,
Serguei
On 2/12/19 11:24 AM, Chris Plummer wrote:
> Hi Gary,
>
> The changes look good.
>
> thanks,
>
> Chris
>
> On 2/12/19 11:04 AM, gary.adams at oracle.com wrote:
>> 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/7977d291/attachment-0001.html>
More information about the serviceability-dev
mailing list