[10] RFR for JDK-8169961: Memory leak after debugging session
Shafi Ahmad
shafi.s.ahmad at oracle.com
Thu Jul 20 08:41:33 UTC 2017
Hi Daniel,
> -----Original Message-----
> From: Langer, Christoph [mailto:christoph.langer at sap.com]
> Sent: Monday, July 17, 2017 6:44 PM
> To: Shafi Ahmad <shafi.s.ahmad at oracle.com>; Daniel Daugherty
> <daniel.daugherty at oracle.com>
> Cc: serviceability-dev at 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-
> > bounces at openjdk.java.net] On Behalf Of Shafi Ahmad
> > Sent: Montag, 17. Juli 2017 14:25
> > To: Daniel Daugherty <daniel.daugherty at oracle.com>; serviceability-
> > dev at 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.ahmad at oracle.com>; serviceability-
> > > dev at 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.ahmad at oracle.com>; serviceability-
> > > >> dev at 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 at slc12kkg:/scratch/shshahma/Java/jdk10-hs$ cd jdk/
shshahma at slc12kkg:/scratch/shshahma/Java/jdk10-hs/jdk$ st
M src/jdk.jdi/share/classes/com/sun/tools/jdi/TargetVM.java
Regards,
Shafi
> > > > Regards,
> > > > Shafi
> > > >
> > > >> Dan
> > > >>
> > > >>> Regards,
> > > >>> Shafi
> > >
More information about the serviceability-dev
mailing list