Community
Participate
Working Groups
3.6 M4 but also 3.5.x. When compare editors are reused (setInput(...) called) then the old compare editor input is not disposed.
This should be backported to 3.5.2.
Old compare editor inputs were supposed to be disposed as soon as the UI for it gets disposed[1]. This has been broken by adding a check for CompareEditor type (see fix for bug 259411 bullet #5). [1] org.eclipse.compare.CompareEditorInput.createContents(...).new DisposeListener() {...}.widgetDisposed(DisposeEvent)
I broke it, I'll fix it.
Created attachment 155492 [details] Fix v01 The fix reverts logical changes introduced by faulty patch from bug 259411. The misconception in that patch was that the compare editor input is disposed only when editor is closed (so we decided to add a check in the dispose listener and manually dispose the input when disposing the editor). This is obviously wrong, because as Dani reported in this bug, the input should be also disposed when reusing compare editors. This fix resembles one of the first patches proposed on bug 259411 where I tried to postpone disposing the input until all UI widgets are released by creating a dummy widget and adding a dispose listener to it. That was an ugly hack, so this time the dispose listener is added to the last widget found in the UI structure. Therefore, compare editor input is (always, the faulty check has been removed) disposed at the end making it possible to refer during widgets disposal. Unfortunately, this is not end of the story. Playing around with the patch I've found two worrying scenarios: 1) Analyzing heap dumps I was able to observe that compare editor inputs are not gc'ed because they are referenced by Editor History even though the editor should not appear in the Editor list when it is closed. I was able to get same results with Eclipse 3.4 so I guess the issue has been there for quite a long time. Moreover, this seems to be more like a bug in the workbench (similar to bug 295046). 2) The second case is much worse. Since 'Use alternative content viewer' feature (bug 201116 and bug 220457) was added after the patch from bug 259411 had been released, switching inputs in the compare editor is based on a faulty disposal implementation. This patch fixes the disposal part but now when switching content viewers 'Widget is disposed' errors are thrown. This must be fixed as well.
Created attachment 155493 [details] mylyn/context/zip
In the second scenario we dispose the UI associated with this compare editor input replacing it with a new one, but at the same time we want to keep the input. In other words, the input stays the same, only the UI changes. Afaik, this is something current implementation is not prepared for.
Created attachment 155542 [details] Fix v02 Patch that fixes the issue from comment 6. The compare editor input is disposed only when the top most widget is disposed. The trick with adding a dispose listener to the last widget is still crucial. Dani, could please you verify if this fixes the leaks? Or were you referring to the compare editor input referenced by Editor History (comment 4, bullet 1)?
Fix v02 applied to HEAD, available in builds >N20100110-2000. When verified, I'll release the same fix to the R3_5_maintenance branch.
Verified that it fixes the problem from comment 0 but did not review the code in detail.
Created attachment 155966 [details] Fix v02 (for 3.5.2) The same patch as "Fix v02" adjusted for 3.5.x backporting.
"Fix v02 (for 3.5.2)" has been applied to R3_5_maintenance branch, I've also increased the bundle version to "3.5.2.qualifier".
*** Bug 288596 has been marked as a duplicate of this bug. ***
