diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/activities/IActivityManager.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/activities/IActivityManager.java index 1cf46d696e8..47e5403d03b 100644 --- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/activities/IActivityManager.java +++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/activities/IActivityManager.java @@ -114,4 +114,13 @@ public interface IActivityManager { * performed. */ void removeActivityManagerListener(IActivityManagerListener activityManagerListener); + + /** + * Disposes this activity manager. Removes all listeners. The behavior of all + * other methods after disposing is undefined. + * + * @since 3.131 + */ + void dispose(); + } diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/Workbench.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/Workbench.java index 82975d14599..69b3e6bd8db 100644 --- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/Workbench.java +++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/Workbench.java @@ -3019,18 +3019,27 @@ private void shutdown() { workbenchListeners.clear(); cancelEarlyStartup(); - if (workbenchService != null) + if (workbenchService != null) { workbenchService.unregister(); + } workbenchService = null; - if (e4WorkbenchService != null) + if (e4WorkbenchService != null) { e4WorkbenchService.unregister(); + } e4WorkbenchService = null; // for dynamic UI registry.removeRegistryChangeListener(extensionEventHandler); registry.removeRegistryChangeListener(startupRegistryListener); + // shut down activity helper before disposing workbench activity support; + // dispose activity support before disposing service locator to avoid + // unnecessary activity disablement processing + activityHelper.shutdown(); + workbenchActivitySupport.dispose(); + WorkbenchHelpSystem.disposeIfNecessary(); + // Bring down all of the services. serviceLocator.dispose(); application.getCommands().removeAll(commandsToRemove); @@ -3038,11 +3047,7 @@ private void shutdown() { getDisplay().removeFilter(SWT.MouseDown, backForwardListener); backForwardListener = null; - workbenchActivitySupport.dispose(); - WorkbenchHelpSystem.disposeIfNecessary(); - // shutdown the rest of the workbench - activityHelper.shutdown(); uninitializeImages(); if (WorkbenchPlugin.getDefault() != null) { WorkbenchPlugin.getDefault().reset(); diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/AbstractActivityManager.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/AbstractActivityManager.java index 6ac7f011aec..dbe1192c63e 100644 --- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/AbstractActivityManager.java +++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/AbstractActivityManager.java @@ -22,6 +22,8 @@ public abstract class AbstractActivityManager implements IActivityManager { private ListenerList activityManagerListeners; + private boolean disposed; + protected AbstractActivityManager() { } @@ -60,4 +62,15 @@ public void removeActivityManagerListener(IActivityManagerListener activityManag activityManagerListeners.remove(activityManagerListener); } } + + @Override + public void dispose() { + activityManagerListeners.clear(); + disposed = true; + } + + protected boolean isDisposed() { + return disposed; + } + } diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/MutableActivityManager.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/MutableActivityManager.java index b24d348165b..33912033976 100644 --- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/MutableActivityManager.java +++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/MutableActivityManager.java @@ -133,6 +133,23 @@ public MutableActivityManager(ITriggerPointAdvisor triggerPointAdvisor, IActivit readRegistry(true); } + @Override + public void dispose() { + super.dispose(); + clearExpressions(); + activityRegistry.removeActivityRegistryListener(activityRegistryListener); + activitiesById.clear(); + activityRequirementBindingsByActivityId.clear(); + activityPatternBindingsByActivityId.clear(); + categoriesById.clear(); + categoryActivityBindingsByCategoryId.clear(); + categoryDefinitionsById.clear(); + definedActivityIds.clear(); + definedCategoryIds.clear(); + enabledActivityIds.clear(); + identifiersById.clear(); + } + @Override synchronized public IActivity getActivity(String activityId) { if (activityId == null) { @@ -596,7 +613,7 @@ private Map updateActivities(Collection activityI } private IPropertyChangeListener enabledWhenListener = event -> { - if (addingEvaluationListener) { + if (addingEvaluationListener || isDisposed()) { return; } diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/ProxyActivityManager.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/ProxyActivityManager.java index dae3408a5b8..fe0de34f1d3 100644 --- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/ProxyActivityManager.java +++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/ProxyActivityManager.java @@ -66,4 +66,5 @@ public Set getEnabledActivityIds() { public IIdentifier getIdentifier(String identifierId) { return activityManager.getIdentifier(identifierId); } + } diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/ws/WorkbenchActivitySupport.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/ws/WorkbenchActivitySupport.java index db28a387b78..47b62414745 100644 --- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/ws/WorkbenchActivitySupport.java +++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/ws/WorkbenchActivitySupport.java @@ -315,11 +315,13 @@ private ImageBindingRegistry getCategoryImageBindingRegistry() { } /** - * Dispose of the image registries. + * Dispose of the image registries and activity manager. * * @since 3.1 */ public void dispose() { + proxyActivityManager.dispose(); + mutableActivityManager.dispose(); if (activityImageBindingRegistry != null) { activityImageBindingRegistry.dispose(); PlatformUI.getWorkbench().getExtensionTracker().unregisterHandler(activityImageBindingRegistry); diff --git a/tests/org.eclipse.ui.tests.rcp/Eclipse RCP Tests/org/eclipse/ui/tests/rcp/RcpTestSuite.java b/tests/org.eclipse.ui.tests.rcp/Eclipse RCP Tests/org/eclipse/ui/tests/rcp/RcpTestSuite.java index f5cff186b15..bf5c9ba393e 100644 --- a/tests/org.eclipse.ui.tests.rcp/Eclipse RCP Tests/org/eclipse/ui/tests/rcp/RcpTestSuite.java +++ b/tests/org.eclipse.ui.tests.rcp/Eclipse RCP Tests/org/eclipse/ui/tests/rcp/RcpTestSuite.java @@ -25,7 +25,7 @@ @RunWith(Suite.class) @Suite.SuiteClasses({ PlatformUITest.class, WorkbenchAdvisorTest.class, WorkbenchConfigurerTest.class, WorkbenchWindowConfigurerTest.class, ActionBarConfigurerTest.class, IWorkbenchPageTest.class, - WorkbenchSaveRestoreStateTest.class, WorkbenchListenerTest.class }) + WorkbenchSaveRestoreStateTest.class, WorkbenchListenerTest.class, WorkbenchTest.class }) public class RcpTestSuite { diff --git a/tests/org.eclipse.ui.tests.rcp/Eclipse RCP Tests/org/eclipse/ui/tests/rcp/WorkbenchTest.java b/tests/org.eclipse.ui.tests.rcp/Eclipse RCP Tests/org/eclipse/ui/tests/rcp/WorkbenchTest.java new file mode 100644 index 00000000000..0d8cf28d7d9 --- /dev/null +++ b/tests/org.eclipse.ui.tests.rcp/Eclipse RCP Tests/org/eclipse/ui/tests/rcp/WorkbenchTest.java @@ -0,0 +1,56 @@ +/******************************************************************************* + * Copyright (c) 2023 Vector Informatik GmbH and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.ui.tests.rcp; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +import org.eclipse.swt.widgets.Display; +import org.eclipse.ui.IWorkbench; +import org.eclipse.ui.PlatformUI; +import org.eclipse.ui.activities.IActivityManager; +import org.eclipse.ui.activities.IActivityManagerListener; +import org.eclipse.ui.application.WorkbenchAdvisor; +import org.eclipse.ui.tests.harness.util.RCPTestWorkbenchAdvisor; +import org.junit.Test; + +public class WorkbenchTest { + /** + * Tests activity manager behavior during workbench shutdown. + * + * See https://github.com/eclipse-platform/eclipse.platform.ui/issues/1084 + */ + @Test + public void testWorkbenchShutdownProducesNoActivityManagerEvents() { + IActivityManagerListener activityListener = mock(IActivityManagerListener.class); + WorkbenchAdvisor closeAfterStartupAdvisor = new RCPTestWorkbenchAdvisor() { + @Override + public void postStartup() { + IWorkbench workbench = getWorkbenchConfigurer().getWorkbench(); + IActivityManager activityManager = workbench.getActivitySupport().getActivityManager(); + activityManager.addActivityManagerListener(activityListener); + workbench.close(); + }; + }; + + runWorkbench(closeAfterStartupAdvisor); + + verify(activityListener, never()).activityManagerChanged(any()); + } + + private void runWorkbench(WorkbenchAdvisor workbenchAdvisor) { + Display display = PlatformUI.createDisplay(); + PlatformUI.createAndRunWorkbench(display, workbenchAdvisor); + } + +} diff --git a/tests/org.eclipse.ui.tests.rcp/META-INF/MANIFEST.MF b/tests/org.eclipse.ui.tests.rcp/META-INF/MANIFEST.MF index 3e4fdebbdd8..5db851bf9ee 100644 --- a/tests/org.eclipse.ui.tests.rcp/META-INF/MANIFEST.MF +++ b/tests/org.eclipse.ui.tests.rcp/META-INF/MANIFEST.MF @@ -7,6 +7,7 @@ Bundle-Vendor: %providerName Require-Bundle: org.eclipse.core.runtime, org.eclipse.ui, org.junit, + org.mockito.mockito-core, org.eclipse.test.performance, org.eclipse.ui.tests.harness Bundle-ActivationPolicy: lazy