From aa864386e10da8ad2c190f8d0cfaa0d1ac0b1102 Mon Sep 17 00:00:00 2001 From: Maxime Porhel Date: Tue, 29 Aug 2023 15:48:27 +0200 Subject: [PATCH] [#89] Avoid deadlocks with Control/Uncontrol handlers Bug: https://github.com/eclipse-sirius/sirius-desktop/issues/89 Signed-off-by: Maxime Porhel --- ...UncontrolWithOpenedRepresentationTest.java | 53 ++++++++----------- .../LockedRepresentationContainerTest.java | 39 +++++++------- .../api/control/SiriusControlHandler.java | 5 +- .../api/control/SiriusUncontrolHandler.java | 5 +- .../control/DesignerControlAction.java | 8 +-- 5 files changed, 50 insertions(+), 60 deletions(-) diff --git a/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/ControlUncontrolWithOpenedRepresentationTest.java b/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/ControlUncontrolWithOpenedRepresentationTest.java index 41b0e6cc2e..78b45234d3 100644 --- a/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/ControlUncontrolWithOpenedRepresentationTest.java +++ b/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/ControlUncontrolWithOpenedRepresentationTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2015 THALES GLOBAL SERVICES. + * Copyright (c) 2015, 2023 THALES GLOBAL SERVICES. * 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 @@ -93,10 +93,9 @@ protected void onSetUpAfterOpeningDesignerPerspective() throws Exception { } /** - * Open a diagram located under a semantic element to control. Once - * controlled, we validate that the diagram is still editable. Next the - * semantic element is uncontrolled and we validate again that the diagram - * is still editable. + * Open a diagram located under a semantic element to control. Once controlled, we validate that the diagram is + * still editable. Next the semantic element is uncontrolled and we validate again that the diagram is still + * editable. * * @throws Exception * Test error. @@ -106,10 +105,9 @@ public void testCheckOpenedDiagramBehaviourOnControlUncontrol() throws Exception } /** - * Open a diagram located under a sub element of a semantic element to - * control. Once controlled, we validate that the diagram is still editable. - * Next the semantic element is uncontrolled and we validate again that the - * diagram is still editable. + * Open a diagram located under a sub element of a semantic element to control. Once controlled, we validate that + * the diagram is still editable. Next the semantic element is uncontrolled and we validate again that the diagram + * is still editable. * * @throws Exception * Test error. @@ -119,10 +117,8 @@ public void testCheckOpenedSubDiagramBehaviourOnControlUncontrol() throws Except } /** - * Open a table located under a semantic element to control. Once - * controlled, we validate that the table is still editable. Next the - * semantic element is uncontrolled and we validate again that the table is - * still editable. + * Open a table located under a semantic element to control. Once controlled, we validate that the table is still + * editable. Next the semantic element is uncontrolled and we validate again that the table is still editable. * * @throws Exception * Test error. @@ -132,10 +128,9 @@ public void testCheckOpenedTableBehaviourOnControlUncontrol() throws Exception { } /** - * Open a table located under a sub element of a semantic element to - * control. Once controlled, we validate that the table is still editable. - * Next the semantic element is uncontrolled and we validate again that the - * table is still editable. + * Open a table located under a sub element of a semantic element to control. Once controlled, we validate that the + * table is still editable. Next the semantic element is uncontrolled and we validate again that the table is still + * editable. * * @throws Exception * Test error. @@ -145,9 +140,8 @@ public void testCheckOpenedSubTableBehaviourOnControlUncontrol() throws Exceptio } /** - * Open a tree located under a semantic element to control. Once controlled, - * we validate that the tree is still editable. Next the semantic element is - * uncontrolled and we validate again that the tree is still editable. + * Open a tree located under a semantic element to control. Once controlled, we validate that the tree is still + * editable. Next the semantic element is uncontrolled and we validate again that the tree is still editable. * * @throws Exception * Test error. @@ -157,10 +151,9 @@ public void testCheckOpenedTreeBehaviourOnControlUncontrol() throws Exception { } /** - * Open a tree located under a sub element of a semantic element to control. - * Once controlled, we validate that the tree is still editable. Next the - * semantic element is uncontrolled and we validate again that the tree is - * still editable. + * Open a tree located under a sub element of a semantic element to control. Once controlled, we validate that the + * tree is still editable. Next the semantic element is uncontrolled and we validate again that the tree is still + * editable. * * @throws Exception * Test error. @@ -224,11 +217,9 @@ private void processCheckOpenedTreeBehaviourOnControlUncontrol(String treeName) } /** - * This method is used to bypass a current issue where the "Control" or - * "Uncontrol" actions are unexpectedly disabled. Therefore - * SWTBotUtils.clickContextMenu can't be used.
- * Note that the first step is to access the package p1 and then to - * control/uncontrol it. + * This method is used to bypass a current issue where the "Control" or "Uncontrol" actions are unexpectedly + * disabled. Therefore SWTBotUtils.clickContextMenu can't be used.
+ * Note that the first step is to access the package p1 and then to control/uncontrol it. * * @param shouldControl * true if we should control, false if we should uncontrol @@ -251,11 +242,11 @@ private void launchControlUncontrolUsingHandler(final boolean shouldControl) { @Override public void run() { try { - final Shell activeShell = PlatformUI.getWorkbench().getDisplay().getActiveShell(); - new ProgressMonitorDialog(activeShell).run(false, false, new WorkspaceModifyOperation() { + new ProgressMonitorDialog(PlatformUI.getWorkbench().getDisplay().getActiveShell()).run(false, false, new WorkspaceModifyOperation() { @Override protected void execute(IProgressMonitor monitor) throws CoreException, InvocationTargetException, InterruptedException { + Shell activeShell = PlatformUI.getWorkbench().getDisplay().getActiveShell(); if (shouldControl) { new SiriusControlHandler().performControl(activeShell, p1, new NullProgressMonitor()); } else { diff --git a/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/LockedRepresentationContainerTest.java b/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/LockedRepresentationContainerTest.java index afcdcbd54a..cfcfea35d3 100644 --- a/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/LockedRepresentationContainerTest.java +++ b/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/LockedRepresentationContainerTest.java @@ -34,9 +34,8 @@ import org.eclipse.swtbot.swt.finder.widgets.SWTBotTreeItem; /** - * Ensure that some actions on representation are disabled when the - * {@link DView} is locked by using the permission authority - * {@link ReadOnlyPermissionAuthority} + * Ensure that some actions on representation are disabled when the {@link DView} is locked by using the permission + * authority {@link ReadOnlyPermissionAuthority} * * @author Mickael LANOE */ @@ -116,8 +115,8 @@ protected void onSetUpAfterOpeningDesignerPerspective() throws Exception { } /** - * Ensure that the creation of a new representation is forbidden when the - * representation container is locked by using permission authority. + * Ensure that the creation of a new representation is forbidden when the representation container is locked by + * using permission authority. */ public void testCreateRepresentation() { SWTBotTreeItem semanticPackageNode = getSelectedSemanticPackageNode(); @@ -134,9 +133,8 @@ public void testCreateRepresentation() { } /** - * Ensure that the creation of a new representation from the session is - * forbidden when the representation container is locked by using permission - * authority. + * Ensure that the creation of a new representation from the session is forbidden when the representation container + * is locked by using permission authority. */ public void testCreateRepresentationFromSession() { SWTBotTreeItem sessionTreeItem = localSession.getRootSessionTreeItem(); @@ -167,9 +165,8 @@ public void testCreateRepresentationFromSession() { } /** - * Ensure that the creation of a new representation from the session is - * forbidden when the representation container is locked by using permission - * authority. + * Ensure that the creation of a new representation from the session is forbidden when the representation container + * is locked by using permission authority. */ public void testControlSemanticModel() { SWTBotTreeItem semanticPackageNode = getSelectedSemanticSubPackageNode(); @@ -210,32 +207,32 @@ public void testControlSemanticModel() { } /** - * Ensure that the deletion of a representation is forbidden when the - * representation container is locked by using permission authority. + * Ensure that the deletion of a representation is forbidden when the representation container is locked by using + * permission authority. */ public void testDeleteRepresentation() { doTestRepresentationAction(DELETE_ACTION); } /** - * Ensure that the copy of a representation is forbidden when the - * representation container is locked by using permission authority. + * Ensure that the copy of a representation is forbidden when the representation container is locked by using + * permission authority. */ public void testCopyRepresentation() { doTestRepresentationAction(COPY_ACTION); } /** - * Ensure that the extract of a representation is forbidden when the - * representation container is locked by using permission authority. + * Ensure that the extract of a representation is forbidden when the representation container is locked by using + * permission authority. */ public void testExtractRepresentation() { doTestRepresentationAction(EXTRACT_TO_AIRD_FILE); } /** - * Ensure that the move of a representation is forbidden when the - * representation container is locked by using permission authority. + * Ensure that the move of a representation is forbidden when the representation container is locked by using + * permission authority. */ public void testMoveRepresentation() { // First create a new ".aird" file to have the "Move" action @@ -258,8 +255,8 @@ public void testMoveRepresentation() { } /** - * Ensure that an action on a representation is forbidden when the - * representation container is locked by using permission authority. + * Ensure that an action on a representation is forbidden when the representation container is locked by using + * permission authority. * * @param action * the action to check diff --git a/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/api/control/SiriusControlHandler.java b/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/api/control/SiriusControlHandler.java index 35c308c619..1ff146dc5b 100644 --- a/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/api/control/SiriusControlHandler.java +++ b/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/api/control/SiriusControlHandler.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2009, 2019 THALES GLOBAL SERVICES and others. + * Copyright (c) 2009, 2023 THALES GLOBAL SERVICES 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 @@ -84,7 +84,8 @@ public Object execute(final ExecutionEvent event) throws ExecutionException { protected void execute(IProgressMonitor monitor) throws CoreException, InvocationTargetException, InterruptedException { try { monitor.beginTask(Messages.SiriusControlHandler_controlTask, 1); - performControl(HandlerUtil.getActiveShell(event), semanticRoot, new SubProgressMonitor(monitor, 1)); + Shell activeShell = PlatformUI.getWorkbench().getDisplay().getActiveShell(); + performControl(activeShell, semanticRoot, new SubProgressMonitor(monitor, 1)); } finally { monitor.done(); } diff --git a/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/api/control/SiriusUncontrolHandler.java b/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/api/control/SiriusUncontrolHandler.java index 0801c945d0..10cb67b63b 100644 --- a/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/api/control/SiriusUncontrolHandler.java +++ b/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/api/control/SiriusUncontrolHandler.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2009, 2019 THALES GLOBAL SERVICES and others. + * Copyright (c) 2009, 2023 THALES GLOBAL SERVICES 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 @@ -68,7 +68,8 @@ public Object execute(final ExecutionEvent event) throws ExecutionException { protected void execute(IProgressMonitor monitor) throws CoreException, InvocationTargetException, InterruptedException { try { monitor.beginTask(Messages.SiriusUncontrolHandler_uncontrolTask, 1); - performUncontrol(HandlerUtil.getActiveShell(event), semanticRoot, new SubProgressMonitor(monitor, 1)); + Shell activeShell = PlatformUI.getWorkbench().getDisplay().getActiveShell(); + performUncontrol(activeShell, semanticRoot, new SubProgressMonitor(monitor, 1)); } finally { monitor.done(); } diff --git a/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/internal/actions/control/DesignerControlAction.java b/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/internal/actions/control/DesignerControlAction.java index 52b1076b99..2b9511e745 100644 --- a/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/internal/actions/control/DesignerControlAction.java +++ b/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/internal/actions/control/DesignerControlAction.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2009, 2015 THALES GLOBAL SERVICES and others. + * Copyright (c) 2009, 2023 THALES GLOBAL SERVICES 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 @@ -55,7 +55,6 @@ public DesignerControlAction() { @Override public void run() { final boolean controlling = this.command == null; - final Shell shell = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getShell(); int choice = ISaveablePart2.YES; final Session session = SessionManager.INSTANCE.getSession(this.eObject); if (session != null) { @@ -77,10 +76,11 @@ protected void execute(IProgressMonitor monitor) throws CoreException, Invocatio monitor.subTask(Messages.DesignerControlAction_savingTask); session.save(new SubProgressMonitor(monitor, 1)); } + Shell activeShell = PlatformUI.getWorkbench().getDisplay().getActiveShell(); if (controlling) { - new SiriusControlHandler().performControl(shell, eObject, new SubProgressMonitor(monitor, 1)); + new SiriusControlHandler().performControl(activeShell, eObject, new SubProgressMonitor(monitor, 1)); } else { - new SiriusUncontrolHandler().performUncontrol(shell, eObject, new SubProgressMonitor(monitor, 1)); + new SiriusUncontrolHandler().performUncontrol(activeShell, eObject, new SubProgressMonitor(monitor, 1)); } } finally { monitor.done();