Community
Participate
Working Groups
I20100802-1800 Debug leaks WorkbenchWindows in multiple ways. After - Window > New Window - switch to Debug perspective - close window there are multiple paths through which a WorkbenchWindow is leaked: - ViewContextManager#windowClosed(..) should call fWindowToService.remove(window) - SourceLookupManager#windowClosed(..) should call fServices.remove(window) - AbstractBreadcrumb#dispose() should remove display listeners, but it's only called by LaunchViewBreadcrumb#dispose() which in turn is never called - maybe more paths
Created attachment 175855 [details] Patch with leak fixes. In addition to the leaks pointed out I also found: - ToggleStepFilter action is never disposed. - PresentationContext doesn't reset fWindow on dispose - ViewContextService doesn't reset fWindow on dispose - SourceLookupService doesn't reset fWindow on dispose The reason that the mentioned hash maps don't remove the reference upon windowClosed() call, as pointed out, is that those managers are often called by clients with a cached reference to the closed window. When that happens the managers end up creating a new service instance for that window and thus leak the window reference anyway. The simplest solution I can think of is to have these managers hold a weak reference to the window instead. Any objections?
Comment on attachment 175855 [details] Patch with leak fixes. I forgot to set the review flag. Darin, could you take a look at these changes and let me know what you think. I'll be away for next three weeks so if you like the changes please go ahead and apply them.
Sure, I'll take a look once M1 has shipped.
Created attachment 176056 [details] patch You can avoid using weak hash maps with this patch. A "null" debug context service is returned when a window has been closed. Not sure which approach is better.
I released the 2nd patch for now. All tests pass.
I'm not crazy about the weak references either, so the null-service approach seems like a good idea to me.
Reviewed.
