Bug 551676 - avoid stack overflow on working set updater restoreI20200331-1800

With the changes from commit f638d309e065aa92f1b3616728e174b5ed7c4488 we
may run into endless loop if the IPropertyChangeListener installed on
AbstractWorkingSetManager trigger working set API's during working set
restore operation.

The recursion is possible because the current working set design has a
major flaw - it allows events to be triggered on not yet fully created
working sets (see bug 479217).

The recursion is possible via this call stack:

WorkingSet.restoreWorkingSet(WorkingSet.java:151)
AbstractWorkingSet.getElementsArray(AbstractWorkingSet.java:166)
AbstractWorkingSet.isEmpty(AbstractWorkingSet.java:200)
BAD_CLIENT.propertyChange()
AbstractWorkingSetManager$5.run(AbstractWorkingSetManager.java:378)
AbstractWorkingSetManager.firePropertyChange(AbstractWorkingSetManager.java:391)
AbstractWorkingSetManager.getUpdater(AbstractWorkingSetManager.java:725)
WorkingSet.getUpdater(WorkingSet.java:314)
WorkingSet.restoreWorkingSet(WorkingSet.java:151)

To avoid the recursion on restore, we fix the getUpdater() so that it
remembers just created working set *before* sending an event to the
listeners. This way the next call to getUpdater() will return already
created instance, instead of creating new one, and recursion does not
occure.

Change-Id: Ibd2ce63dbf4497b403f83aa6a88a17747c18c629
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>

1 files changed, 13 insertions, 3 deletions

diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/AbstractWorkingSetManager.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/AbstractWorkingSetManager.java
index c26d6d02efd..2d7ca9f9aec 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/AbstractWorkingSetManager.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/AbstractWorkingSetManager.java
@@ -716,19 +716,29 @@ public abstract class AbstractWorkingSetManager extends EventManager
 	}
 
 	IWorkingSetUpdater getUpdater(WorkingSetDescriptor descriptor) {
-		IWorkingSetUpdater updater = updaters.get(descriptor.getId());
+		String updaterId = descriptor.getId();
+		IWorkingSetUpdater updater = updaters.get(updaterId);
 		if (updater == null) {
 			updater = descriptor.createWorkingSetUpdater();
+			boolean fireChange = true;
 			if (updater == null) {
 				updater = NULL_UPDATER;
-			} else {
+				fireChange = false;
+			}
+			synchronized (updaters) {
+				IWorkingSetUpdater old = updaters.putIfAbsent(updaterId, updater);
+				if (fireChange) {
+					// don't fire again if already installed
+					fireChange = old == null;
+				}
+			}
+			if (fireChange) {
 				firePropertyChange(CHANGE_WORKING_SET_UPDATER_INSTALLED, null, updater);
 				PlatformUI.getWorkbench().getExtensionTracker().registerObject(
 						descriptor.getConfigurationElement().getDeclaringExtension(), updater,
 						IExtensionTracker.REF_WEAK);
 
 			}
-			updaters.put(descriptor.getId(), updater);
 		}
 		return updater;
 	}