Community
Participate
Working Groups
Build 2.0.2 and 2.1 Test Case 1: Ensure Outline view is visible. Open 10 Java editors and then close all with the "Close All" action. ==> 9 editors stay in memory. Test Case 2: Ensure Outline view is visible. Open 2 Java editors via Package Explorer Activated the second editor Activated the first editor Press the close box of the second editor without activating it before Press the close box of the first editor ==> both editors stay in memory We suspect that PageBookView.partClosed does not dispose the local action bar and does not remove the listeners for non-active parts.
After ensuring that dispose is called on subActionBars in PageBookView.partClosed and PageBookView.dispose all editors were correctly removed from memory.
Dani, could you please include the patch you describe in comment #1?
Should fix for 2.0.2.
Sure, will be there in a minute, just need to kick off 2.0.2. NOTE: I only did a quick fix. There's also listeners and other stuff that might be worth being removed/disposed.
Created attachment 2336 [details] First cut of patch - need refinement
The preferred change is to dispose the action bars in removePage, and change partClosed to the following. public void partClosed(IWorkbenchPart part) { // Update the active part. if (activeRec != null && activeRec.part == part) { showPageRec(defaultPageRec); } // Find and remove the part page. PageRec rec = getPageRec(part); if (rec != null) removePage(rec); } However, this has a different order of method invocations. The current order is: - dispose the sub action bar of old page - activate new (default) page The new order would be: - deactivate sub action bar contributions for old page - activate new (default) page - dispose sub action bar of old page (this wouldn't cause any extra UI updates though, since the action bars are only updated once). It would be safer to go with Dani's change for 2.0.2.
I agree with Nick. I also first thought removePage would be the right place when I notice the change in control flow. What about the selection listener? Shouldn't it be removed too? At least the last page might leak the listener if it wasn't activated, right?
The selection listener is only hooked on the active page, and is removed in both partClosed and showPageRec (showPageRec also adds it to the new active page). But it looks like it's not removed in dispose(). I'll have a look. Or were you thinking of a different case?
It's OK not to unhook the selection listener on dispose, since everything is going away: the page book view, its site, all its pages and their sites. I didn't see any leaks here when profiling this.
Fixed in r202-v20021107a.
Nick, I thought of the following scenario: I remove the last editor but it was not active. It would go through partClosed, then not being treated because the part is not the active one. But I guess this case i.e. no part ever being active - does not exist, right? Another issue (though not critical): it seems that the property change listener on the subActionBar isn't removed.
Is there a new PR for the 2.1 stream? I didn't get new code when I catched up this morning.
I forgot to release it in the 2.1 stream. I'd prefer to release the fix in comment #6 though. Reopening to do this.
I'll look at the other cases you mention too.
I've applied the change in comment #6 in the HEAD stream. For the case 1 in comment #11, I tried: - open A.java - selection listener gets added to A's outline page - open B.java - selection listener gets removed from A's outline page - selection listener gets added to B's outline page - close A.java - no change to selection listeners - deactivate B by clicking on Navigator - no change to selection listeners (the Navigator does not provide an outline, so it still shows B's) - close B without activating it - selection listener gets removed from B's outline page (as far as the outline view is concerned, it was still the active page) So this case looks fine. Or were you imagining some other scenario? For case 2, it should not matter if the property change listener is not removed, since the SubActionBars instance is being let go. However, it would be cleaner to clear the listener list, in case the SubActionBars is hung onto by someone else for some reason. That way it won't hold a reference to the PageBookView. I've added a propertyChangeListeners.clear() to SubActionBars.dispose() (in HEAD stream only).
Note that JavaOutlinePage.dispose() could use fSelectionChangedListeners.clear () instead of iterating.
changed code in JavaOutlinePage. Thanks for the tip.
