From 145a426e8da1abc4e5d4971473d3750e720275b9 Mon Sep 17 00:00:00 2001 From: Vasili Gulevich Date: Mon, 22 Apr 2024 22:41:27 +0400 Subject: [PATCH] Clean caches upon deletion of a Target Definition #1246 TargetPlatformHelper and TargetPlatformService keep separate caches for target definition, resulting in a leak of TargetDefinition objects. The leak happens when TargetDefinition.resolve() caches data in TargetPlatformHelper. This also allows to skip the explicit removal of a target selected for removal from the TargetPlatformHelper's cache in TargetPlatformPreferencePage.handleRemove() because the removal will done when the change is applied and TargetPlatformService.deleteTarget() is then called. Fixes https://github.com/eclipse-pde/eclipse.pde/issues/1246 --- .../core/target/TargetPlatformService.java | 2 ++ .../tests/target/LocalTargetDefinitionTests.java | 16 ++++++++++++++++ .../TargetPlatformPreferencePage.java | 3 --- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/TargetPlatformService.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/TargetPlatformService.java index 38d682a272..2d8459c3e5 100644 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/TargetPlatformService.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/TargetPlatformService.java @@ -74,6 +74,7 @@ import org.eclipse.pde.internal.core.PDECore; import org.eclipse.pde.internal.core.PDEPreferencesManager; import org.eclipse.pde.internal.core.TargetDefinitionManager; +import org.eclipse.pde.internal.core.TargetPlatformHelper; import org.osgi.service.prefs.BackingStoreException; /** @@ -156,6 +157,7 @@ public void deleteTarget(ITargetHandle handle) throws CoreException { fExtTargetHandles.remove(((ExternalFileTargetHandle) handle).getLocation()); } ((AbstractTargetHandle) handle).delete(); + TargetPlatformHelper.getTargetDefinitionMap().remove(handle); } @Override diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/LocalTargetDefinitionTests.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/LocalTargetDefinitionTests.java index b85445ef11..8e461f8748 100644 --- a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/LocalTargetDefinitionTests.java +++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/LocalTargetDefinitionTests.java @@ -765,6 +765,22 @@ public void testArguments() throws Exception { } + /** + * Test for https://github.com/eclipse-pde/eclipse.pde/issues/1246 + */ + @Test + public void testDeleteCleansCaches() throws Exception { + ITargetDefinition definition = getNewTarget(); + try { + assertFalse(TargetPlatformHelper.getTargetDefinitionMap().containsKey(definition.getHandle())); + definition.resolve(null); + assertTrue(TargetPlatformHelper.getTargetDefinitionMap().containsKey(definition.getHandle())); + } finally { + getTargetService().deleteTarget(definition.getHandle()); + assertFalse(TargetPlatformHelper.getTargetDefinitionMap().containsKey(definition.getHandle())); + } + } + /** * Tests that a single (lower) version of a bundle can be included in the * target platform. diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/preferences/TargetPlatformPreferencePage.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/preferences/TargetPlatformPreferencePage.java index 1466d4d549..158a22aed5 100644 --- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/preferences/TargetPlatformPreferencePage.java +++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/preferences/TargetPlatformPreferencePage.java @@ -744,9 +744,6 @@ private void handleRemove() { } fRemoved.addAll(selected); fTargets.removeAll(selected); - for (ITargetDefinition element : selected) { - TargetPlatformHelper.getTargetDefinitionMap().remove(element.getHandle()); - } // Quick hack because the first refresh loses the checkedState, which is being used to bold the active target fTableViewer.refresh(false); fTableViewer.refresh(true);