Bugzilla – Full Text Bug Listing
| Summary: | [Compatibility] Eclipse context of PageSites are never disposed | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] e4 | Reporter: | Remy Suen <remy.suen> | ||||
| Component: | UI | Assignee: | Remy Suen <remy.suen> | ||||
| Status: | RESOLVED FIXED | QA Contact: | Remy Suen <remy.suen> | ||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | bokowski, john.arthorne, ob1.eclipse, pwebster | ||||
| Version: | 1.0 | Flags: | pwebster:
review+
ob1.eclipse: review+ |
||||
| Target Milestone: | 4.1 M1 | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
I think we should consider this for the 7 p.m. build. The current behaviour means that those contexts will exist for every editor that was opened, for as long as the outline view is open in any perspective. PW Oleg or John, could you look at the code for this (we're just matching up a eclipseContext.dispose() with the object that does the create). Boris, could you consider for 4.0? PW Oleg, could you please do an additional review? > + e4Context.dispose();
In this particular case this is not going to do much of a difference.
The only "external" thing hapenning on #dipose() is that whoever was injected with the context will be notified. However, look at how this context is used:
serviceLocator.setContext(e4Context);
and
IHandlerService handlerService = new LegacyHandlerService(e4Context);
So, while it might be used down the road for injection from either ServiceLocator, or the LegacyHandlerService, there is no immediate point where it is injected anywhere.
From the "internal side, the parent context keeps child contexts as a weak reference, so in theory, using #dispose() won't [directly] help garbage collection.
And, as a side reference, this is what LegacyHandlerService#dispose() looks like:
public void dispose() {
E4Util.message("LegacyHandlerService.dispose: should it do something?"); //$NON-NLS-1$
}
To summarize:
This is a right thing to do, but not now, not in RC4, unless there is a *very* demonstrable benefit.
Moving to 4.1. We're leaking like a sieve anyway (bug 320857). +1 for 4.1. Fix released for 4.1. |
Created attachment 175215 [details] PageSite patch v1 We activate/deactivate them properly but never actually dispose them _individually_. So technically speaking, they're just going to lie around forever until you actually close the view. And keeping shared parts in mind, you'd have to close them in every single perspective you have up.