JBIDE-20366: Check for missing m2e connectors after import #38
Conversation
|
needs @fbricon rivew |
| import org.eclipse.ui.wizards.datatransfer.ProjectConfigurator; | ||
|
|
||
| public class MavenProjectConfigurator implements ProjectConfigurator { | ||
|
|
||
| private static class CumulativeMappingDiscoveryJob extends MappingDiscoveryJob { | ||
| private static CumulativeMappingDiscoveryJob INSTANCE; | ||
| private Set<IProject> toProcess; |
There was a problem hiding this comment.
memory leak: toProcess keeps project references in job instance, after import, and keeps on piling new projects
There was a problem hiding this comment.
Thanks for noticing it. I'll fix it as part of next iteration.
More than the small memory leak (which is actually size of a pointer * max number of projects in workspace), I believe this can have an important impact on the performance of the connector discovery operations.
|
Updated patch to clear the toProcess set. |
| return Status.OK_STATUS; | ||
| } | ||
|
|
||
| public void addProjects(Collection<IProject> projects) { |
There was a problem hiding this comment.
since this is a public method, I'd recommend to check for null input
|
The latest version of this patch adds a null check. |
|
@fbricon : could you please merge it if it's fine for you (both branch and master) ? |
|
@fbricon Any objection to merge it in both branch and master (I can do it by myself, but would like someone to +1 ) |
| } | ||
|
|
||
| public void addProjects(Collection<IProject> projects) { | ||
| if (projects != null) { |
There was a problem hiding this comment.
I think this method should check the job status before adding new projects. If the instance is already running, gets past openProposalWizard(), projects will be added to the set that'll be cleared right after.
So basically you should throw an IllegalStateException if the job is running while you're adding projects
|
Updated change as advised. |
|
@fbricon can you please review and merge before code freeze? |
| if (this.started) { | ||
| throw new IllegalStateException("Cannot add projects when processing is started"); | ||
| } | ||
| if (projects != null) { |
There was a problem hiding this comment.
projects can't be null, it's instantiated in the constructor of this singleton
|
Ok to apply once toProcess.clear() is moved to a try{} finally{} block |
|
Done and merged. |
No description provided.