Community
Participate
Working Groups
Build Identifier: Build id: M20100909-0800 Start and termiante GDB debug session. See that there are leaks of org.eclipse.cdt.dsf.gdb.internal.ui.viewmodel.launch.LaunchVMProvider that don't get garbadge collected. Reproducible: Always Steps to Reproduce: Start and termiante GDB debug session. See that there are leaks of org.eclipse.cdt.dsf.gdb.internal.ui.viewmodel.launch.LaunchVMProvider that don't get garbadge collected.
Created attachment 183947 [details] We need to unistall the proxy... This is not sufficient... I'll keep looking for other places...
Created attachment 183951 [details] Missing things in GdbAdapterFactory I also found stuff in GdbAdapterFactory. But I still see leaks.
With the first patch (LaunchRootVMNode) I decreased the reference to DefaultVMModelProxyStrategy to two. The next two I think are coming from: AbstractCachingVMProvider (line 921) The DefaultVMModelProxyStrategy is added to fActiveModelProxies but is never removed. I expected at some point that instance to be removed from the list fActiveModelProxies by calling rootElementRemovedFromCache(Object rootElement). At this moment I am not too familiar with AbstractCachingVMProvider. I hope someone that has played more with this class give us a suggestion. I'll keep digging...
*** cdt cvs genie on behalf of mkhouzam *** Bug 331232: Missing some unregistering [*] GdbAdapterFactory.java 1.16 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/GdbAdapterFactory.java?root=Tools_Project&r1=1.15&r2=1.16
(In reply to comment #2) > Created an attachment (id=183951) [details] > Missing things in GdbAdapterFactory > > I also found stuff in GdbAdapterFactory. But I still see leaks. I committed this patch to HEAD and 7_0
*** cdt cvs genie on behalf of mkhouzam *** Bug 331232: Missing unregisters [*] GdbAdapterFactory.java 1.15.2.1 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/GdbAdapterFactory.java?root=Tools_Project&r1=1.15&r2=1.15.2.1
(In reply to comment #6) > *** cdt cvs genie on behalf of mkhouzam *** > Bug 331232: Missing unregisters > > [*] GdbAdapterFactory.java 1.15.2.1 > http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/GdbAdapterFactory.java?root=Tools_Project&r1=1.15&r2=1.15.2.1 Hi Mark, Did you notice whether this would re-introduce bug 293109?
(In reply to comment #7) > (In reply to comment #6) > > *** cdt cvs genie on behalf of mkhouzam *** > > Bug 331232: Missing unregisters > > > > [*] GdbAdapterFactory.java 1.15.2.1 > > http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/GdbAdapterFactory.java?root=Tools_Project&r1=1.15&r2=1.15.2.1 > > Hi Mark, > Did you notice whether this would re-introduce bug 293109? I tried it and the first time it failed! The DV was missing an update. But every other time after that, it worked properly (about 20 times). Why would this change affect the Terminate and Relaunch?
Calling session.unregisterModelAdapter(ILaunch.class); means that clients can't retrieve the launch object from an IDMContext. I thought Terminate and Relaunch may be affected by this, but it appears that it's not. Thanks for checking.
Created attachment 195562 [details] Fix memory leaks I did a little investigation triggered by bug 345164 and found a few leaks which prevent the cleanup of DsfSession and Spawner objects after a DSF-GDB launch is terminated and removed from the Debug view. - Remove reference to executor instance from DefaultDsfExecutor.fThreadToExecutorMap when the executor terminates - Avoid reference to DsfSession in AbstractDMContext, use session id instead - Make GDBContainerDMC a static class in GDBProcesses_7_0 - Remove TracingConsole as service event listener on dispose The only remaining leak (AFAICT) is the DefaultVMModelProxyStrategy which is covered by bug 345476. With this patch applied and when I close the Debug view after the session is terminated (which disposes the model proxy), I can find no longer an instance of DsfSession or Spawner in the heap dump.
Reference to DsfSession in AbstractDMContext was specifically used to keep the DsfSession alive, until there's AbstractDMContext that can access it. Unless there's some other clever solution to that, I believe this patch will reintroduce https://bugs.eclipse.org/bugs/show_bug.cgi?id=293109
(In reply to comment #11) > Reference to DsfSession in AbstractDMContext was specifically used to keep the > DsfSession alive, until there's AbstractDMContext that can access it. Unless > there's some other clever solution to that, I believe this patch will > reintroduce https://bugs.eclipse.org/bugs/show_bug.cgi?id=293109 Thanks, that's an interesting aspect. The main issue I saw with DMCs is that the editor annotation model holds references to the last annotation change. I.e. if the last change was removing the IP annotation, this annotation is held back and with it a IFrameDMContext and DsfSession. Removing the DsfSession reference seemed easier than fixing the annotation model.
Thanks Toni, that is really awesome! Just a note that if you enable the Reverse debugging buttons, there is one more leak in ReverseToggleCommandHandler, which I gather I haven't done such a good job with. I'll try to get to it later today, unless you want to take care of it earlier. org.eclipse.cdt.debug.internal.ui.commands.ReverseToggleCommandHandler@0x746f5518 (37 bytes) (field fActiveContext:) --> org.eclipse.cdt.dsf.ui.viewmodel.datamodel.AbstractDMVMNode$DMVMContext@0x75b1a8e0 (20 bytes) (field fNode:) --> org.eclipse.cdt.dsf.debug.ui.viewmodel.launch.StackFramesVMNode@0x750c8448 (33 bytes) (field fSession:) --> org.eclipse.cdt.dsf.service.DsfSession@0x750ae010 (32 bytes)
What do you say to committing the changes except the ones for AbstractDMContext.java? I guess we can deal with AbstractDMContext.java in the next release, or do you have an idea?
(In reply to comment #14) > What do you say to committing the changes except the ones for > AbstractDMContext.java? Done. The remaining changes should be safe. > I guess we can deal with AbstractDMContext.java in the next release, or do you > have an idea? I think I'll file a bug with Platform/Text for the annotation model issue. As for AbstractDMContext, I think it would make sense to separate model adapters from the DsfSession object, but that would need some API changes (similar to bug 252859).
*** cdt cvs genie on behalf of aleherbau *** Bug 331232 - There are memory leaks with each GDB debug session (partial fix) [*] DefaultDsfExecutor.java 1.12 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/concurrent/DefaultDsfExecutor.java?root=Tools_Project&r1=1.11&r2=1.12 [*] GDBProcesses_7_0.java 1.54 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses_7_0.java?root=Tools_Project&r1=1.53&r2=1.54 [*] TracingConsole.java 1.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/TracingConsole.java?root=Tools_Project&r1=1.1&r2=1.2
(In reply to comment #15) > (In reply to comment #14) > > What do you say to committing the changes except the ones for > > AbstractDMContext.java? > > Done. The remaining changes should be safe. Thanks!
Created attachment 195723 [details] Fix for ReverseToggleCommandHandler I have committed this fix for the leak in ReverseToggleCommandHandler.
*** cdt cvs genie on behalf of aleherbau *** Bug 331232 - There are memory leaks with each GDB debug session (partial fix) [*] ReverseToggleCommandHandler.java 1.5 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.debug.ui/src/org/eclipse/cdt/debug/internal/ui/commands/ReverseToggleCommandHandler.java?root=Tools_Project&r1=1.4&r2=1.5
(In reply to comment #18) > Created attachment 195723 [details] > Fix for ReverseToggleCommandHandler > > I have committed this fix for the leak in ReverseToggleCommandHandler. Thanks Toni, I had to give my attention to some Restart issues for the RC1 build.
