[tests] close some leaked Shells #2615

#2615
eclipse-platform · Dec 11, 2024 · bbebe70 · bbebe70
1 parent 51df26c
commit bbebe70
Show file tree

Hide file tree

Showing 10 changed files with 162 additions and 38 deletions.
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/SwtLeakTestWatcher.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/SwtLeakTestWatcher.java
@@ -0,0 +1,58 @@
+/*******************************************************************************
+ * Copyright (c) 2000, 2023 IBM Corporation 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
+ *
+ * Contributors:
+ *     IBM Corporation - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.ui.tests;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+
+import org.eclipse.swt.widgets.Shell;
+import org.eclipse.ui.IWorkbench;
+import org.eclipse.ui.PlatformUI;
+import org.junit.Assert;
+import org.junit.rules.TestWatcher;
+import org.junit.runner.Description;
+
+public class SwtLeakTestWatcher extends TestWatcher {
+
+	IWorkbench workbench;
+	Set<Shell> preExistingShells;
+
+	@Override
+	protected void starting(Description description) {
+		workbench = PlatformUI.getWorkbench();
+		preExistingShells = Set.of(workbench.getDisplay().getShells());
+		super.starting(description);
+	}
+
+	@Override
+	protected void finished(Description description) {
+		// Check for shell leak.
+		List<String> leakedModalShellTitles = new ArrayList<>();
+		Shell[] shells = workbench.getDisplay().getShells();
+		for (Shell shell : shells) {
+			if (!shell.isDisposed() && !preExistingShells.contains(shell)) {
+				leakedModalShellTitles.add(shell.getText());
+				// closing shell may introduce "not disposed" errors in next tests :
+				// shell.close();
+			}
+		}
+		if (!leakedModalShellTitles.isEmpty()) {
+			Assert.fail(description.getClassName() + "." + description.getDisplayName()
+					+ " Test leaked modal shell(s): [" + String.join(", ", leakedModalShellTitles) + "]");
+		}
+		super.finished(description);
+	}
+
+}
diff --git a/...i.tests/Eclipse UI Tests/org/eclipse/ui/tests/concurrency/NestedSyncExecDeadlockTest.java b/...i.tests/Eclipse UI Tests/org/eclipse/ui/tests/concurrency/NestedSyncExecDeadlockTest.java
@@ -29,16 +29,22 @@
 import org.eclipse.swt.widgets.Display;
 import org.eclipse.swt.widgets.Shell;
 import org.eclipse.ui.actions.WorkspaceModifyOperation;
+import org.eclipse.ui.tests.SwtLeakTestWatcher;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TestWatcher;
 
 /**
  * This is a regression test for a case where a recursive attempt to syncExec
  * from within code that owns a lock would cause deadlock. See bug 76378 for details.
  */
 public class NestedSyncExecDeadlockTest {
 
+	@Rule
+	public TestWatcher swtLeakTestWatcher = new SwtLeakTestWatcher();
+
 	private static class ResourceListener implements IResourceChangeListener {
 		@Override
 		public void resourceChanged(IResourceChangeEvent event) {
@@ -53,7 +59,8 @@ public void resourceChanged(IResourceChangeEvent event) {
 	private final IWorkspace workspace = ResourcesPlugin.getWorkspace();
 
 	public void doTest(final long timeToSleep) throws Exception {
-		ProgressMonitorDialog dialog = new ProgressMonitorDialog(new Shell());
+		Shell shell = new Shell();
+		ProgressMonitorDialog dialog = new ProgressMonitorDialog(shell);
 		dialog.run(true, false, new WorkspaceModifyOperation() {
 			@Override
 			public void execute(final IProgressMonitor pm) {
@@ -77,6 +84,7 @@ public void execute(final IProgressMonitor pm) {
 				});
 			}
 		});
+		shell.close();
 	}
 
 	@Before

diff --git a/...org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/concurrency/TestBug108162.java b/...org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/concurrency/TestBug108162.java
@@ -26,7 +26,10 @@
 import org.eclipse.jface.dialogs.ProgressMonitorDialog;
 import org.eclipse.swt.widgets.Shell;
 import org.eclipse.ui.actions.WorkspaceModifyOperation;
+import org.eclipse.ui.tests.SwtLeakTestWatcher;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TestWatcher;
 
 /**
  * Tests the following sequence of events:
@@ -38,6 +41,10 @@
  * This test asserts that the exception is thrown and that deadlock does not occur.
  */
 public class TestBug108162 {
+
+	@Rule
+	public TestWatcher swtLeakTestWatcher = new SwtLeakTestWatcher();
+
 	static class LockAcquiringOperation extends WorkspaceModifyOperation {
 		@Override
 		public void execute(final IProgressMonitor pm) {
@@ -52,8 +59,9 @@ public void execute(final IProgressMonitor pm) {
 	 */
 	@Test
 	public void testBug() throws CoreException {
+		Shell shell = new Shell();
 		workspace.run((IWorkspaceRunnable) monitor -> {
-			ProgressMonitorDialog dialog = new ProgressMonitorDialog(new Shell());
+			ProgressMonitorDialog dialog = new ProgressMonitorDialog(shell);
 			try {
 				dialog.run(true, false, new LockAcquiringOperation());
 				// should not succeed
@@ -63,5 +71,6 @@ public void testBug() throws CoreException {
 				// lock.
 			}
 		}, workspace.getRoot(), IResource.NONE, null);
+		shell.close();
 	}
 }
diff --git a/...org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/concurrency/TestBug269121.java b/...org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/concurrency/TestBug269121.java
@@ -29,7 +29,10 @@
 import org.eclipse.swt.widgets.Shell;
 import org.eclipse.ui.actions.WorkspaceModifyOperation;
 import org.eclipse.ui.progress.UIJob;
+import org.eclipse.ui.tests.SwtLeakTestWatcher;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TestWatcher;
 
 import junit.framework.AssertionFailedError;
 
@@ -56,6 +59,9 @@
  */
 public class TestBug269121 {
 
+	@Rule
+	public TestWatcher swtLeakTestWatcher = new SwtLeakTestWatcher();
+
 	@Test
 	public void testBug() throws InterruptedException,
 			InvocationTargetException {
@@ -73,8 +79,9 @@ protected void execute(IProgressMonitor monitor) {
 				status.set(0, TestBarrier2.STATUS_DONE);
 			}
 		};
+		Shell shell = new Shell();
 		final ProgressMonitorDialog dialog = new ProgressMonitorDialog(
-				new Shell());
+				shell);
 		Job statusJob = new Job("Checking for deadlock") {
 			@Override
 			protected IStatus run(IProgressMonitor monitor) {
@@ -104,5 +111,6 @@ protected IStatus run(IProgressMonitor monitor) {
 		}
 		job.join();
 		assertTrue("Timeout occurred - possible Deadlock. See logging!", statusJob.getResult().isOK());
+		shell.close();
 	}
 }
diff --git a/.../org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/concurrency/TestBug98621.java b/.../org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/concurrency/TestBug98621.java
@@ -28,7 +28,10 @@
 import org.eclipse.swt.widgets.Display;
 import org.eclipse.swt.widgets.Shell;
 import org.eclipse.ui.actions.WorkspaceModifyOperation;
+import org.eclipse.ui.tests.SwtLeakTestWatcher;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TestWatcher;
 
 /**
  * Tests the following sequence of events:
@@ -47,6 +50,10 @@
  *  @since 3.2
  */
 public class TestBug98621 {
+
+	@Rule
+	public TestWatcher swtLeakTestWatcher = new SwtLeakTestWatcher();
+
 	class TransferTestOperation extends WorkspaceModifyOperation {
 		@Override
 		public void execute(final IProgressMonitor pm) {
@@ -80,8 +87,9 @@ public void threadChange(Thread thread) {
 	 */
 	@Test
 	public void testBug() throws CoreException {
+		Shell shell = new Shell();
 		workspace.run((IWorkspaceRunnable) monitor -> {
-			ProgressMonitorDialog dialog = new ProgressMonitorDialog(new Shell());
+			ProgressMonitorDialog dialog = new ProgressMonitorDialog(shell);
 			try {
 				dialog.run(true, false, new TransferTestOperation());
 			} catch (InvocationTargetException e1) {
@@ -91,5 +99,6 @@ public void testBug() throws CoreException {
 				// ignore
 			}
 		}, workspace.getRoot(), IResource.NONE, null);
+		shell.close();
 	}
 }
diff --git a/...e.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/dialogs/DeprecatedUIPreferencesAuto.java b/...e.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/dialogs/DeprecatedUIPreferencesAuto.java
@@ -13,6 +13,8 @@
  *******************************************************************************/
 package org.eclipse.ui.tests.dialogs;
 
+import static org.junit.Assume.assumeNotNull;
+
 import org.eclipse.jface.dialogs.Dialog;
 import org.eclipse.jface.preference.IPreferenceNode;
 import org.eclipse.jface.preference.PreferenceDialog;
@@ -21,11 +23,17 @@
 import org.eclipse.ui.PlatformUI;
 import org.eclipse.ui.internal.IWorkbenchHelpContextIds;
 import org.eclipse.ui.internal.WorkbenchPlugin;
+import org.eclipse.ui.tests.SwtLeakTestWatcher;
 import org.eclipse.ui.tests.harness.util.DialogCheck;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TestWatcher;
 
 public class DeprecatedUIPreferencesAuto {
 
+	@Rule
+	public TestWatcher swtLeakTestWatcher = new SwtLeakTestWatcher();
+
 	protected Shell getShell() {
 		return DialogCheck.getShell();
 	}
@@ -122,23 +130,22 @@ public void testProjectReferencesProp() {
 	@Test
 	public void testFieldEditorEnablePref() {
 
-		PreferenceDialogWrapper dialog = null;
 		PreferenceManager manager = WorkbenchPlugin.getDefault().getPreferenceManager();
-		if (manager != null) {
-			dialog = new PreferenceDialogWrapper(getShell(), manager);
-			dialog.create();
-
-			for (IPreferenceNode node : manager.getElements(PreferenceManager.PRE_ORDER)) {
-				if (node.getId().equals("org.eclipse.ui.tests.dialogs.EnableTestPreferencePage")) {
-					dialog.showPage(node);
-					EnableTestPreferencePage page = (EnableTestPreferencePage) dialog.getPage(node);
-					page.flipState();
-					page.flipState();
-					break;
-				}
+		assumeNotNull(manager);
+		PreferenceDialogWrapper dialog = new PreferenceDialogWrapper(
+				PlatformUI.getWorkbench().getActiveWorkbenchWindow().getShell(), manager);
+		dialog.create();
+
+		for (IPreferenceNode node : manager.getElements(PreferenceManager.PRE_ORDER)) {
+			if (node.getId().equals("org.eclipse.ui.tests.dialogs.EnableTestPreferencePage")) {
+				dialog.showPage(node);
+				EnableTestPreferencePage page = (EnableTestPreferencePage) dialog.getPage(node);
+				page.flipState();
+				page.flipState();
+				break;
 			}
 		}
-
+		dialog.close();
 	}
 
 }
diff --git a/...org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/dialogs/UIPreferencesAuto.java b/...org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/dialogs/UIPreferencesAuto.java
@@ -13,6 +13,8 @@
  *******************************************************************************/
 package org.eclipse.ui.tests.dialogs;
 
+import static org.junit.Assume.assumeNotNull;
+
 import org.eclipse.jface.dialogs.Dialog;
 import org.eclipse.jface.preference.IPreferenceNode;
 import org.eclipse.jface.preference.PreferenceDialog;
@@ -21,11 +23,16 @@
 import org.eclipse.ui.PlatformUI;
 import org.eclipse.ui.internal.IWorkbenchHelpContextIds;
 import org.eclipse.ui.internal.WorkbenchPlugin;
+import org.eclipse.ui.tests.SwtLeakTestWatcher;
 import org.eclipse.ui.tests.harness.util.DialogCheck;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TestWatcher;
 
 public class UIPreferencesAuto {
 
+	@Rule
+	public TestWatcher swtLeakTestWatcher = new SwtLeakTestWatcher();
 	protected Shell getShell() {
 		return DialogCheck.getShell();
 	}
@@ -124,25 +131,23 @@ public void testProjectReferencesProp() {
 	 */
 	@Test
 	public void testFieldEditorEnablePref() {
-
-		PreferenceDialogWrapper dialog = null;
 		PreferenceManager manager = WorkbenchPlugin.getDefault()
 				.getPreferenceManager();
-		if (manager != null) {
-			dialog = new PreferenceDialogWrapper(getShell(), manager);
-			dialog.create();
-
-			for (IPreferenceNode node : manager.getElements(PreferenceManager.PRE_ORDER)) {
-				if (node.getId().equals("org.eclipse.ui.tests.dialogs.EnableTestPreferencePage")) {
-					dialog.showPage(node);
-					EnableTestPreferencePage page = (EnableTestPreferencePage) dialog.getPage(node);
-					page.flipState();
-					page.flipState();
-					break;
-				}
+		assumeNotNull(manager);
+		PreferenceDialogWrapper dialog = new PreferenceDialogWrapper(
+				PlatformUI.getWorkbench().getActiveWorkbenchWindow().getShell(), manager);
+		dialog.create();
+
+		for (IPreferenceNode node : manager.getElements(PreferenceManager.PRE_ORDER)) {
+			if (node.getId().equals("org.eclipse.ui.tests.dialogs.EnableTestPreferencePage")) {
+				dialog.showPage(node);
+				EnableTestPreferencePage page = (EnableTestPreferencePage) dialog.getPage(node);
+				page.flipState();
+				page.flipState();
+				break;
 			}
 		}
-
+		dialog.close();
 	}
 
 }
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/dialogs/UIWizardsAuto.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/dialogs/UIWizardsAuto.java
@@ -39,16 +39,22 @@
 import org.eclipse.ui.internal.dialogs.NewWizard;
 import org.eclipse.ui.internal.ide.IIDEHelpContextIds;
 import org.eclipse.ui.internal.wizards.newresource.ResourceMessages;
+import org.eclipse.ui.tests.SwtLeakTestWatcher;
 import org.eclipse.ui.tests.harness.util.DialogCheck;
 import org.eclipse.ui.tests.harness.util.FileUtil;
 import org.eclipse.ui.wizards.newresource.BasicNewFileResourceWizard;
 import org.eclipse.ui.wizards.newresource.BasicNewFolderResourceWizard;
 import org.eclipse.ui.wizards.newresource.BasicNewProjectResourceWizard;
 import org.junit.After;
 import org.junit.Ignore;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TestWatcher;
 
 public class UIWizardsAuto {
+	@Rule
+	public TestWatcher swtLeakTestWatcher = new SwtLeakTestWatcher();
+
 	private static final int SIZING_WIZARD_WIDTH = 470;
 
 	private static final int SIZING_WIZARD_HEIGHT = 550;
@@ -332,7 +338,8 @@ private void checkWizardWindowTitle(String windowTitle) {
 
 		initNewWizard(newWizard);
 
-		WizardDialog dialog = new WizardDialog(getShell(), newWizard);
+		WizardDialog dialog = new WizardDialog(PlatformUI.getWorkbench().getActiveWorkbenchWindow().getShell(),
+				newWizard);
 		dialog.create();
 
 		if(windowTitle == null) {