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

Bug 255005

Summary: [Progress] IWorkbenchSiteProgressService never removes its IJobChangeListener instance
Product: [Eclipse Project] Platform Reporter: Remy Suen <remy.suen>
Component: UIAssignee: Prakash Rangaraj <prakash>
Status: RESOLVED DUPLICATE QA Contact:
Severity: normal    
Priority: P3 CC: pwebster
Version: 3.5   
Target Milestone: ---   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
Sample ViewPart to reproduce the problem.
none
Patch with leak tests and a potential fix. none

Description Remy Suen CLA 2008-11-12 05:13:58 EST
Created attachment 117642 [details]
Sample ViewPart to reproduce the problem.

When a job is scheduled via a workbench part's IWorkbenchSiteProgressService, an IJobChangeListener is attached to the job to provide UI level notifications to the user that the workbench part is currently processing something. This is done by italicizing the text in the workbench part's tab item (and optionally altering the presentation of your mouse's cursor). Unfortunately, this listener is never removed (I guess this would qualify as an object leak?). This causes the problem where scheduling that same job in the future will _always_ send notifications to that workbench part.

Please see the attached sample view for reproducing the error. You can schedule the job by clicking on the button in the view. You will see your cursor change to the "half busy" cursor and the part title is italicized. This is fine as we are scheduling through the progress service. Now upon the conclusion of the job, it schedules itself to be run again in three seconds. When the three seconds is up, you will see the cursor and part title change again even though it wasn't scheduled through the progress service!
Comment 1 Remy Suen CLA 2008-11-14 01:34:05 EST
Created attachment 117862 [details]
Patch with leak tests and a potential fix.

This patch will add tests to our LeakTests test class to prevent future regressions. There is also a hack to remove the listener upon the conclusion of the done(IJobChangeEvent). I'm quite sure it's a horrible way of programming to remove the listener when it's being notified so I imagine we don't want to commit this. Feel free to use it for illustrative purposes though.
Comment 2 Remy Suen CLA 2008-11-16 05:28:24 EST
A better way of solving this might be to just create one single IJobChangeListener and attach it to the IJobManager instance instead. When a job is about to be run or is completed, we can check if this job has been scheduled through the progress service or not (some kind of set or map will keep track of this) and react accordingly if necessary. On dispose(), we can remove the listener from the job manager and then that'll be that.
Comment 3 Prakash Rangaraj CLA 2009-01-29 04:57:11 EST
This will be addressed as part of Bug#254984. I've added Remy's leak test to the patch in that bug.

Marking this as duplicate.

*** This bug has been marked as a duplicate of bug 254984 ***