Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 369015 - [Sync View] DiffTreeChangesSection memory leak
Summary: [Sync View] DiffTreeChangesSection memory leak
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Team (show other bugs)
Version: 3.7.1   Edit
Hardware: All All
: P3 major (vote)
Target Milestone: 3.8 M6   Edit
Assignee: Tomasz Zarna CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 352282
  Show dependency tree
 
Reported: 2012-01-18 19:06 EST by Marc-André Laperle CLA
Modified: 2012-02-07 04:20 EST (History)
1 user (show)

See Also:

Attachments
DiffTreeChangesSection memory leak patch (2.19 KB, patch)
2012-01-18 19:07 EST, Marc-André Laperle CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.