Hi Shafi,

I'm reviewing it.

Thanks,
Serguei


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

Regards,
Shafi



-----Original Message-----
From: Shafi Ahmad
Sent: Wednesday, July 26, 2017 8:26 AM
To: serviceability-dev@openjdk.java.net
Cc: Roger Calnan <roger.cal...@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@openjdk.java.net
Subject: RE: [10] RFR for JDK-8169961: Memory leak after debugging
session

Hi,

-----Original Message-----
From: Langer, Christoph [mailto:christoph.lan...@sap.com]
Sent: Monday, July 17, 2017 9:01 PM
To: Poonam Parhar <poonam.ba...@oracle.com>
Cc: Shafi Ahmad <shafi.s.ah...@oracle.com>; serviceability-
d...@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.

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


Reply via email to