Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 369015

Summary: [Sync View] DiffTreeChangesSection memory leak
Product: [Eclipse Project] Platform Reporter: Marc-André Laperle <malaperle>
Component: TeamAssignee: Tomasz Zarna <tomasz.zarna>
Status: VERIFIED FIXED QA Contact:
Severity: major    
Priority: P3 CC: remy.suen
Version: 3.7.1   
Target Milestone: 3.8 M6   
Hardware: All   
OS: All   
Whiteboard:
Bug Depends on:    
Bug Blocks: 352282    
Attachments:
Description Flags
DiffTreeChangesSection memory leak patch none

Description Marc-André Laperle CLA 2012-01-18 19:06:55 EST 
Build id: M20110909-1335

1. Open the Synchronize view, do a Sychronize with patch
2. Remove synchronization
3. Repeat steps 1 and 2 several times to make the issue more apparent
4. Remove All Finishes operations in the Progress view. (This will workaround another occasional memory problem I've seen where the swt.Link holds a reference to a RefreshParticipantJob, I will open a separate bug).
5. Run GC, wait for a bit.
6. JobManager will still hold a references to the DiffTreeChangesSections, this can be seen with Eclipse Memory Analyzer

The leak is much more severe using EGit (see Bug 352282) but the bug is the same.

In MAT:

1. Search for .*ApplyPatchSubscriber.* in Dominator. 
2. Right-click on one of the org.eclipse.team.internal.ui.synchronize.patch.ApplyPatchSubscriber
3. Merge Shortest paths to GC root, Exclude all phantom/weak/soft

org.eclipse.core.internal.jobs.InternalWorker @ 0x160e5318  Worker-JM Native Stack, Thread
'- manager org.eclipse.core.internal.jobs.JobManager @ 0x15edb090
   '- jobListeners org.eclipse.core.internal.jobs.JobListeners @ 0x15edb410
      '- global org.eclipse.core.runtime.ListenerList @ 0x15ee7340
         '- listeners java.lang.Object[5] @ 0x1e53b588
            '- [2] org.eclipse.team.internal.ui.mapping.DiffTreeChangesSection$1 @ 0x1e4b3c38
               '- this$0 org.eclipse.team.internal.ui.mapping.DiffTreeChangesSection @ 0x1e4b3c50

Solution: DiffTreeChangesSection adds a JobChangeAdapter in its constructor but doesn't remove it. It should remove it in its dispose() method.
Comment 1 Marc-André Laperle CLA 2012-01-18 19:07:55 EST 
Created attachment 209711 [details]
DiffTreeChangesSection memory leak patch
Comment 2 Tomasz Zarna CLA 2012-01-26 08:54:17 EST 
Thanks for the descriptive summary and the patch Marc! I will try to have a look at it next week.
Comment 3 Tomasz Zarna CLA 2012-01-31 12:18:33 EST 
Fixed with 62be766bed8025810531fe026ab3b1e233904b06, available in builds >= N20120131-2000. Thanks again Marc, nice catch!
Comment 4 Tomasz Zarna CLA 2012-02-06 08:10:35 EST 
Marc, could you verify the fix using one of the latest N-builds?
Comment 5 Marc-André Laperle CLA 2012-02-07 03:15:34 EST 
(In reply to comment #4)
> Marc, could you verify the fix using one of the latest N-builds?

I tested with eclipse-SDK-N20120205-2000 and it looks good. I don't see any more instances of ApplyPatchSubscriber accumulating because of this. I also tested with EGit 1.3.0.201202051215 and the behavior is similar. I will create separate bugs for the 2 remaining issues mentioned in bug 352282 comment 5 once I have done further investigation.
Comment 6 Tomasz Zarna CLA 2012-02-07 04:20:49 EST 
Awesome, thanks again. Marking the bug as verified.