Community
Participate
Working Groups
Build ID: I20071213-1700 Steps To Reproduce: I have an RCP app that uses the CommonNavigator and I don't use any decorations. So in this case, the listenerList in the DecorationScheduler is empty. However, the removedListeners are never cleared in that case and accumulate, causing a leak when the view is created and recreated. More information:
Created attachment 87944 [details] Patch to I20071213-1500 This patch seems to fix the leak, as measured by the profiler. And in testing it with my IDE it does not seem to do anything bad.
Created attachment 87991 [details] Better Patch to I20071213-1500 Turns out there was more leakage than I originally found. This patch seems to fix all of the problems of holding on to objects. It needs to be looked at carefully though, I added a resetState() method and call it when I think the state of the update thread needs to be reset, but it might be doing too much sometimes. In any case, this patch seems to work fine in my IDE and does not leak when running my junit tests (as long as I have a 200ms delay between each test case to give all of the threads of the DecorationScheduler a chance to run -- sigh).
Created attachment 88018 [details] Another patch Looking at your code the only place where we aren't clearing the removedListeners list where you are is when the listeners length is 0 (which sounds like your case). Here is a simpler patch that I think solves your problem without all of the other resets.
Thanks Todd, what you propose is the patch I started with. Unfortunately this does not solve the entire problem. The pendingUpdate list also hangs onto objects in these conditions, and the resultCache is not properly reset. The patch I provide causes all of these to be properly reset and has been verified (in my tests) to not leak. (In reply to comment #3) > Created an attachment (id=88018) [details] > Another patch > > Looking at your code the only place where we aren't clearing the > removedListeners list where you are is when the listeners length is 0 (which > sounds like your case). Here is a simpler patch that I think solves your > problem without all of the other resets. >
Thanks Francis. I have looked through the code and see you point. Long lived caches are never a good idea anyways so I have released your patch with minor changes for build > 20080129. If you could test against your test case I would be most appreciative.
I picked up your changes from HEAD and they work great! Thanks Tod for your fast work on this. It's been pleasure doing business with you.
And thanks for your contribution and talking me through it. I have added you to the coontributors list in the class comment as well.
Verified by Francis' usecase
Jason please look at a test suite for this
