Test API check: move synchronized inside resourceChanged body

[vogella]

Test PR against vogella/master to verify whether moving synchronized from the public resourceChanged method modifier to an inner synchronized (this) block satisfies API Tools without bumping the bundle minor version. The semantic locking pairing with the parent class (CloseResourceAction.resourceChanged and getSelectedResources) is preserved.

If green, the same change will be applied to PR eclipse-platform#3941 against eclipse-platform/eclipse.platform.ui.

[gemini-code-assist]

Code Review

This pull request introduces a caching mechanism for the project graph in CloseUnrelatedProjectsAction to optimize performance and includes a new suite of enablement tests. While the performance improvements are beneficial, the implementation introduces several critical issues: a memory leak due to an unmanaged workspace listener, data races in getSelectedResources, and a potential deadlock caused by calling selectionChanged within a synchronized block. Additionally, the debounce timing changes in AnimationManager appear unrelated to the core objective and require justification. I have provided feedback on these issues to ensure the stability and correctness of the implementation.

I am having trouble creating individual review comments. Click here to see my feedback.

bundles/org.eclipse.ui.ide/extensions/org/eclipse/ui/actions/CloseUnrelatedProjectsAction.java (194)

Registering the action as a workspace resource change listener in the constructor without a mechanism to unregister it (e.g., by implementing a dispose method or relying on a lifecycle manager) will cause a memory leak. Each instance of this action will remain in the workspace's listener list indefinitely, preventing the action and its associated resources from being garbage collected.

bundles/org.eclipse.ui.ide/extensions/org/eclipse/ui/actions/CloseUnrelatedProjectsAction.java (243-250)

The getSelectedResources method accesses and modifies shared state (oldSelection, projectsToClose) and uses the cached projectGraph without synchronization. Since resourceChanged modifies this state from a background workspace thread, this creates a data race. Additionally, the DisjointSet used in computeRelated is not thread-safe as it performs path compression during findSet calls. This method should be synchronized to ensure thread safety and visibility.

	protected List<? extends IResource> getSelectedResources() {
		synchronized (this) {
			List<? extends IResource> newSelection = super.getSelectedResources();
			if (!oldSelection.equals(newSelection)) {
				oldSelection = newSelection;
				projectsToClose = computeRelated(newSelection);
			}
			return projectsToClose;
		}
	}

bundles/org.eclipse.ui.ide/extensions/org/eclipse/ui/actions/CloseUnrelatedProjectsAction.java (271-276)

Calling selectionChanged while holding a lock on this is a deadlock risk because it triggers listener notifications. Notifications should be performed outside of synchronized blocks to avoid holding locks while executing external code that might attempt to acquire other locks.

				boolean updateSelection = false;
				synchronized (this) {
					invalidateProjectGraph();
					updateSelection = selectionIsOfType(IResource.PROJECT);
				}
				if (updateSelection) {
					selectionChanged(getStructuredSelection());
				}

bundles/org.eclipse.e4.ui.progress/src/org/eclipse/e4/ui/progress/internal/AnimationManager.java (43)

This change increases the debounce delay from 100ms to 200ms. This appears to be an unrelated change to the stated purpose of the PR (moving synchronized blocks) and may affect UI responsiveness. If this was intended to fix a specific issue, it should be documented in the PR description.

bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/progress/AnimationManager.java (31)

This change increases the debounce delay from 100ms to 200ms, which is unrelated to the PR's goal and may impact UI responsiveness.