On 7/20/17 2:41 AM, Shafi Ahmad wrote:
Hi Daniel,
-----Original Message-----
From: Langer, Christoph [mailto:christoph.lan...@sap.com]
Sent: Monday, July 17, 2017 6:44 PM
To: Shafi Ahmad <shafi.s.ah...@oracle.com>; Daniel Daugherty
<daniel.daughe...@oracle.com>
Cc: serviceability-dev@openjdk.java.net
Subject: RE: [10] RFR for JDK-8169961: Memory leak after debugging session
Hi Shafi, Daniel,
In my initial review I oversaw the usage of vm inside the constructor. The
new webrev looks even better. I guess the main fix is the termination of the
thread anyway.
Best regards
Christoph
-----Original Message-----
From: serviceability-dev [mailto:serviceability-dev-
boun...@openjdk.java.net] On Behalf Of Shafi Ahmad
Sent: Montag, 17. Juli 2017 14:25
To: Daniel Daugherty <daniel.daughe...@oracle.com>; serviceability-
d...@openjdk.java.net
Subject: RE: [10] RFR for JDK-8169961: Memory leak after debugging
session
Hi Daniel,
Thank you for the review.
-----Original Message-----
From: Daniel D. Daugherty
Sent: Friday, July 14, 2017 6:46 AM
To: Shafi Ahmad <shafi.s.ah...@oracle.com>; serviceability-
d...@openjdk.java.net
Subject: Re: [10] RFR for JDK-8169961: Memory leak after debugging
session
On 7/13/17 10:19 AM, Shafi Ahmad wrote:
Hi Daniel,
Thank you for the review.
-----Original Message-----
From: Daniel D. Daugherty
Sent: Thursday, July 13, 2017 7:40 PM
To: Shafi Ahmad <shafi.s.ah...@oracle.com>; serviceability-
d...@openjdk.java.net
Subject: Re: [10] RFR for JDK-8169961: Memory leak after
debugging session
On 7/13/17 2:45 AM, Shafi Ahmad wrote:
Hi,
Please review the code change for the fix of bug 'JDK-8169961:
Memory
leak after debugging session'
Summary:
1. It seems that the thread created for
com.sun.tools.jdi.TargetVM.EventController is never stopped and
keeps
a hard reference to the VirtualMachineImpl. This leads to
VirtualMachineImpl leak after debug session is finished.
EventController is private class and member field vm of type
VirtualMachineImpl, holding the hard reference to the
VirtualMachineImpl. I am not seeing the usage of filed vm so we
can
remove it safely.
2. Added eventController.release(); before 'Target VM interface
thread
exiting'
3. TargetVM gets an EventController which is a daemon thread,
but don't
see the thread having a way of stopping so added code to exit as
soon as TargetVM thread stops listening.
jdk10 bug: https://bugs.openjdk.java.net/browse/JDK-8169961
webrev link:
http://cr.openjdk.java.net/~shshahma/8169961/webrev.00/
src/jdk.jdi/share/classes/com/sun/tools/jdi/TargetVM.java
L330: VirtualMachineImpl vm;
L335: this.vm = vm;
This comment caught my eye:
> I am not seeing the usage of filed vm so we can remove it
safely.
So you're deleting the 'vm' instance variable, but:
L363: JDWP.VirtualMachine.HoldEvents.process(vm);
L365: JDWP.VirtualMachine.ReleaseEvents.process(vm);
would use it if it was still there...
Where did the run() method find a 'vm' variable? I'm guessing
the one in the TargetVM outer class:
L46: private VirtualMachineImpl vm;
Okay so we don't have to pass (and save a copy of
VirtualMachineImpl).
So this constructor:
L333: EventController(VirtualMachineImpl vm) {
No longer uses the 'vm' parameter at all and you
can remove the parameter and update the caller.
332 private class EventController extends Thread {
333 int controlRequest = 0;
334
335 EventController(VirtualMachineImpl vm) {
336 super(vm.threadGroupForJDI(), "JDI Event Control Thread");
337 setDaemon(true);
338 setPriority((MAX_PRIORITY + NORM_PRIORITY)/2);
339 super.start();
340 }
We can't remove formal parameter 'vm' as this is referenced at line#
336.
So the reference to 'vm' on L336 won't resolve to the one in the
TargetVM outer class:
L46: private VirtualMachineImpl vm;
like L363 and L365 do...
Yes, you are right as this is an inner class constructor.
Please find updated webrev.
http://cr.openjdk.java.net/~shshahma/8169961/webrev.01/
Regards,
Shafi
Dan
Testing: run jprt and I provided the FBP and it works for them.
JPRT doesn't execute any JDI related tests. You need to run the
NSK JDI test suite and the com/sun/jdi tests in the JDK.
I will run NSK JDI test and update this thread with the results.
I have run the nsk.jdi tests and all are fine.
TEST 429/1062 429/1062 nsk/jdi/ReferenceType/visibleMethods/visibmethod003 PASS
TEST 646/1062 646/1062 nsk/jdi/ClassUnloadRequest/addClassFilter/filter001 PASS
TEST 648/1062 648/1062
nsk/jdi/ClassUnloadRequest/addClassExclusionFilter/exclfilter001 PASS
HARNESS ENDED: Thu Jul 20 01:02:31 PDT 2017
TOTAL TESTS IN RUN: 1062
TEST PASS: 1062; 100% rate
TEST FAIL: 0; 0% rate
TEST UNDEFINED: 0; 0% rate
TEST INCOMPLETE: 0; 0% rate
TESTS NOT RUN: 0
TOTAL TEST IN TESTLIST: 1142
Finished building target 'test-hotspot-closed-tonga-only' in configuration
'linux-x64'
shshahma@slc12kkg:/scratch/shshahma/Java/jdk10-hs$ cd jdk/
shshahma@slc12kkg:/scratch/shshahma/Java/jdk10-hs/jdk$ st
M src/jdk.jdi/share/classes/com/sun/tools/jdi/TargetVM.java
Thanks for doing the extra testing.
Dan
Regards,
Shafi
Regards,
Shafi
Dan
Regards,
Shafi
