[545853] Avoid a possible dead lock while selecting new created elements

In org.eclipse.sirius.ui.tools.internal.editor.SelectDRepresentationElementsListener.resourceSetChanged(ResourceSetChangeEvent),
we indirectly perform a Display.syncExec to retrieve the current active
editor (org.eclipse.sirius.common.ui.tools.api.util.EclipseUIUtil.getActiveEditor()).
As we are retaining the current EMF Transaction (the transaction lock is
released once the postCommit() is executed) that could potentially cause
a deadlock.
For instance, when performing a manual refresh on a diagram, new
graphical elements can be created. In the case of Capella for instance,
the org.polarsys.capella.core.ui.properties.CapellaDataListenerForPropertySections
is notified and schedule the refresh of the properties view within the
UI thread. This refresh try to acquire the lock on the transaction but
this one is kept by the Sirius refresh job and this job is waiting for
the ui thread in
electDRepresentationElementsListener.resourceSetChanged.
More generally, we should never wait for the UI thread in a thread that
has a lock on some resources.

Bug: 545853
Change-Id: I7c13e15eada6ab2f6ef3a446de5cc1546fd6eb45
Signed-off-by: Florian Barbin <florian.barbin@obeo.fr>

2 files changed, 56 insertions, 10 deletions

diff --git a/plugins/org.eclipse.sirius.tests.junit.support/src/org/eclipse/sirius/tests/support/api/AbstractToolDescriptionTestCase.java b/plugins/org.eclipse.sirius.tests.junit.support/src/org/eclipse/sirius/tests/support/api/AbstractToolDescriptionTestCase.java
index 9fa37c6f15..8e522d60b2 100644
--- a/plugins/org.eclipse.sirius.tests.junit.support/src/org/eclipse/sirius/tests/support/api/AbstractToolDescriptionTestCase.java
+++ b/plugins/org.eclipse.sirius.tests.junit.support/src/org/eclipse/sirius/tests/support/api/AbstractToolDescriptionTestCase.java
@@ -1,5 +1,5 @@
 /**
- * Copyright (c) 2015 Obeo
+ * Copyright (c) 2015, 2019 Obeo
  * 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
@@ -57,9 +57,24 @@ public abstract class AbstractToolDescriptionTestCase extends SiriusTestCase {
      *            semantic object at the root of selected elements
      */
     protected void checkExpectedElementsInSelection(DialectEditor editor, List<String> objectsNameToCheck, int expectedSize, boolean semanticObjectName) {
-        List<DSemanticDecorator> selections = new ArrayList<DSemanticDecorator>(DialectUIManager.INSTANCE.getSelection(editor));
 
-        assertEquals("Bad selection size, after tool execution.", expectedSize, selections.size());
+        // The Job performing the selection might not have finished. To make the test more reliable, we try to
+        // synchronize with the UI Thread several times until the expected selection is reached, or we fell into
+        // timeout.
+        List<DSemanticDecorator> selections = new ArrayList<DSemanticDecorator>();
+        TestsUtil.waitUntil(new ICondition() {
+            @Override
+            public boolean test() throws Exception {
+                TestsUtil.synchronizationWithUIThread();
+                selections.clear();
+                selections.addAll(DialectUIManager.INSTANCE.getSelection(editor));
+                return expectedSize == selections.size();
+            }
+            @Override
+            public String getFailureMessage() {
+                return "Bad selection size, after tool execution.";
+            }
+        });
 
         if (objectsNameToCheck != null) {
             for (int i = 0; i < objectsNameToCheck.size(); i++) {
diff --git a/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/internal/editor/SelectDRepresentationElementsListener.java b/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/internal/editor/SelectDRepresentationElementsListener.java
index d20736606c..30b19b4d1f 100644
--- a/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/internal/editor/SelectDRepresentationElementsListener.java
+++ b/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/internal/editor/SelectDRepresentationElementsListener.java
@@ -18,6 +18,8 @@ import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 
 import org.eclipse.emf.common.notify.Notification;
 import org.eclipse.emf.ecore.EObject;
@@ -40,6 +42,7 @@ import org.eclipse.sirius.viewpoint.DRepresentation;
 import org.eclipse.sirius.viewpoint.DRepresentationElement;
 import org.eclipse.sirius.viewpoint.UIState;
 import org.eclipse.sirius.viewpoint.ViewpointPackage;
+import org.eclipse.swt.widgets.Display;
 import org.eclipse.ui.IEditorPart;
 
 import com.google.common.base.Preconditions;
@@ -108,15 +111,43 @@ public class SelectDRepresentationElementsListener extends ResourceSetListenerIm
         // Do not react to undo/redo. The option might not be present: react if
         // value is Boolean.FALSE or null ie is not Boolean.TRUE
         if (!Boolean.TRUE.equals(event.getTransaction().getOptions().get(Transaction.OPTION_IS_UNDO_REDO_TRANSACTION))) {
-            IEditorPart activeEditor = EclipseUIUtil.getActiveEditor();
-            if (dialectEditor.equals(activeEditor)) {
-                DRepresentation currentRep = dialectEditor.getRepresentation();
+            DRepresentation currentRep = dialectEditor.getRepresentation();
+            Display display = Display.getCurrent();
+
+            // We are the UI Thread, so we perform the selection.
+            if (display != null && Thread.currentThread() == display.getThread()) {
+                IEditorPart activeEditor = EclipseUIUtil.getActiveEditor();
+                if (dialectEditor.equals(activeEditor)) {
+                    List<DRepresentationElement> elementsToSelect = extractElementsToSelect(event, currentRep);
+                    if (elementsToSelect != null) {
+                        // Set the selection in async exec: for some dialect, ui
+                        // could be refresh by another post commit triggered after
+                        // this one and doing some UI refresh in sync exec.
+                        EclipseUIUtil.displayAsyncExec(new SetSelectionRunnable(dialectEditor, elementsToSelect));
+                    }
+                }
+            }
+            // We are out of the UI Thread.
+            else {
+                // We retrieve the elementToSelect before since we cannot differ this call in a separate thread.
                 List<DRepresentationElement> elementsToSelect = extractElementsToSelect(event, currentRep);
                 if (elementsToSelect != null) {
-                    // Set the selection in async exec: for some dialect, ui
-                    // could be refresh by another post commit triggered after
-                    // this one and doing some UI refresh in sync exec.
-                    EclipseUIUtil.displayAsyncExec(new SetSelectionRunnable(dialectEditor, elementsToSelect));
+                    // The current EMF transaction (we are in a post commit listener here) is not released until all
+                    // post commits are executed. If we are out of the UI Thread, retrieving the current active editor
+                    // implies to wait for the UI Thread. To avoid a dead lock by blocking the current transaction while
+                    // waiting for the UI Thread (this one could also be waiting for the transaction to be released) we
+                    // execute this code in a separate thread.
+                    ExecutorService executorService = Executors.newSingleThreadExecutor();
+                    executorService.submit(() -> {
+                        // getActiveEditor will perform a synExec.
+                        IEditorPart activeEditor = EclipseUIUtil.getActiveEditor();
+                        if (dialectEditor.equals(activeEditor)) {
+                            // Set the selection in async exec: for some dialect, ui
+                            // could be refresh by another post commit triggered after
+                            // this one and doing some UI refresh in sync exec.
+                            EclipseUIUtil.displayAsyncExec(new SetSelectionRunnable(dialectEditor, elementsToSelect));
+                        }
+                    });
                 }
             }
         }