[10] RFR for JDK-8169961: Memory leak after debugging session

[Daniel D. Daugherty]

On 8/9/17 3:05 AM, Shafi Ahmad wrote:
> May I get it reviewed by  serviceability team.

I don't count as being on the Serviceability team anymore, but
I've pinged Serguei Spitsyn who is back from his vacation...

More below...


>
> Regards,
> Shafi
>
>
>
>> -----Original Message-----
>> From: Shafi Ahmad
>> Sent: Wednesday, July 26, 2017 8:26 AM
>> To: serviceability-dev at openjdk.java.net
>> Cc: Roger Calnan <roger.calnan at oracle.com>
>> Subject: RE: [10] RFR for JDK-8169961: Memory leak after debugging session
>>
>> May I get it reviewed by someone from serviceability group.
>>
>> Webrev link: http://cr.openjdk.java.net/~shshahma/8169961/webrev.01/
>> This review thread: http://mail.openjdk.java.net/pipermail/serviceability-
>> dev/2017-July/021538.html
>>
>> Regards,
>> Shafi
>>
>>> -----Original Message-----
>>> From: Shafi Ahmad
>>> Sent: Monday, July 24, 2017 12:52 PM
>>> To: serviceability-dev at openjdk.java.net
>>> Subject: RE: [10] RFR for JDK-8169961: Memory leak after debugging
>>> session
>>>
>>> Hi,
>>>
>>>> -----Original Message-----
>>>> From: Langer, Christoph [mailto:christoph.langer at sap.com]
>>>> Sent: Monday, July 17, 2017 9:01 PM
>>>> To: Poonam Parhar <poonam.bajaj at oracle.com>
>>>> Cc: Shafi Ahmad <shafi.s.ahmad at oracle.com>; serviceability-
>>>> dev at openjdk.java.net
>>>> Subject: RE: [10] RFR for JDK-8169961: Memory leak after debugging
>>>> session
>>>>
>>>> Hi Poonam,
>>>>
>>>>
>>>>> Line 182: Here, eventController.release() is called after the 'vm'
>>>>> is
>>> disposed.
>>>>> And eventController.release() causes the following statement to be
>>>>> executed on the eventcontroller thread after the 'vm' is disposed:
>>>>>
>>>>> JDWP.VirtualMachine.ReleaseEvents.process(vm);
>>>>>
>>>>> Which does not seem to be right. Someone from the Serviceability
>>>>> group can confirm the correctness of this change.
>>>> I think this is okay, because with the new change shouldListen() is
>>>> called right after the thread returns from wait(). And this will
>>>> lead to the thread immediately exiting.
>>>> JDWP.VirtualMachine.ReleaseEvents.process(vm);
>>>> should not be called in this case.

I just re-read the webrev and I agree with Christoph that this new check:

   L358:                         if (!shouldListen) {
   L359:                            return;
   L360:                         }

will keep the EventController.run() method from trying to
use the 'vm' that has been disposed.

Dan


>>>>
>>>>> Line 330: Instance variable 'VirtualMachineImpl vm' is removed
>>>>> from the EventController class. It is being used further down in
>>>>> its
>>>>> run() method. So I think it cannot be removed.
>>>> The vm object is used from the outer class TargetVM, as
>>>> EventController is an inner class of it.
>>>>
>>>> So in my view it's all correct but still somebody of the
>>>> serviceability group might know better...
>>> Could someone from serviceability group review this.
>>>
>>> Regards,
>>> Shafi
>>>
>>>> Best regards
>>>> Christoph
>>>>