Dispose activity management during shutdown completely and early #1084

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();
+
 }

bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/Workbench.java

@@ -3019,30 +3019,35 @@ 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);
 		application.getCategories().removeAll(categoriesToRemove);
 		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();

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<IActivityManagerListener> 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;
+	}
+
 }

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<String, ActivityEvent> updateActivities(Collection<String> activityI
 	}
 
 	private IPropertyChangeListener enabledWhenListener = event -> {
-		if (addingEvaluationListener) {
+		if (addingEvaluationListener || isDisposed()) {
 			return;
 		}
 

bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/activities/ProxyActivityManager.java

@@ -66,4 +66,5 @@ public Set<String> getEnabledActivityIds() {
 	public IIdentifier getIdentifier(String identifierId) {
 		return activityManager.getIdentifier(identifierId);
 	}
+
 }

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);

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 {
 
 

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);
+	}
+
+}

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