Bug 501681 - Deadlock in WorkbenchErrorHandler.handleM20161026-0400M20161019-0640M20161019-0400M20161013-0730M20161012-1220M20161012-1050M20161012-0900M20161012-0600

Fix a deadlock situation introduced via bug 241244 commit
1904f477320dac4a9f4d45f7be478efba4a0b735.

If a job code wants to handle a status in a "blocking" way, submit an
async task for UI thread to avoid deadlocks.

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

7 files changed, 178 insertions, 22 deletions

diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/statushandlers/StatusManager.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/statushandlers/StatusManager.java
index 0e232ef40e1..a84203ed237 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/statushandlers/StatusManager.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/statushandlers/StatusManager.java
@@ -54,8 +54,8 @@ import org.eclipse.ui.progress.IProgressConstants;
  * <li>SHOW - a style indicating that handlers should show a problem to an user
  * without blocking the calling method while awaiting user response. This is
  * generally done using a non modal {@link Dialog}</li>
- * <li>BLOCK - a style indicating that the handling should block the calling
- * method until the user has responded. This is generally done using a modal
+ * <li>BLOCK - a style indicating that the handling should block the UI
+ * until the user has responded. This is generally done using a modal
  * window such as a {@link Dialog}</li>
  * </ul>
  * </p>
@@ -102,7 +102,7 @@ public class StatusManager {
 	 * therefore likely but not required that the <code>StatusHandler</code>
 	 * would achieve this through the use of a modal dialog.
 	 * </p><p>Due to the fact
-	 * that use of <code>BLOCK</code> will block any thread, care should be
+	 * that use of <code>BLOCK</code> will block UI, care should be
 	 * taken in this use of this flag.
 	 * </p>
 	 */
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/statushandlers/WorkbenchErrorHandler.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/statushandlers/WorkbenchErrorHandler.java
index 30a35a21acf..4fed5d88150 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/statushandlers/WorkbenchErrorHandler.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/statushandlers/WorkbenchErrorHandler.java
@@ -48,22 +48,7 @@ public class WorkbenchErrorHandler extends AbstractStatusHandler {
 			if (Display.getCurrent() != null) {
 				showStatusAdapter(statusAdapter, block);
 			} else {
-				if (block) {
-					Display.getDefault().syncExec(new Runnable() {
-						@Override
-						public void run() {
-							showStatusAdapter(statusAdapter, true);
-						}
-					});
-
-				} else {
-					Display.getDefault().asyncExec(new Runnable() {
-						@Override
-						public void run() {
-							showStatusAdapter(statusAdapter, false);
-						}
-					});
-				}
+				Display.getDefault().asyncExec(() -> showStatusAdapter(statusAdapter, block));
 			}
 		}
 
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/concurrency/FreezeMonitor.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/concurrency/FreezeMonitor.java
new file mode 100644
index 00000000000..9bff4647190
--- /dev/null
+++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/concurrency/FreezeMonitor.java
@@ -0,0 +1,64 @@
+/*******************************************************************************
+ * Copyright (c) 2016 Google, Inc and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *   Stefan Xenos (Google) - Initial implementation
+ *******************************************************************************/
+package org.eclipse.ui.tests.concurrency;
+
+import java.lang.management.ManagementFactory;
+import java.lang.management.ThreadInfo;
+import java.lang.management.ThreadMXBean;
+
+import org.eclipse.core.runtime.ICoreRunnable;
+import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.OperationCanceledException;
+import org.eclipse.core.runtime.jobs.Job;
+
+public class FreezeMonitor {
+
+	private static /* @Nullable */ Job monitorJob;
+
+	public static void expectCompletionIn(final long millis) {
+		done();
+		monitorJob = Job.create("FreezeMonitor", new ICoreRunnable() {
+			@Override
+			public void run(IProgressMonitor monitor) {
+				if (monitor.isCanceled()) {
+					throw new OperationCanceledException();
+				}
+				StringBuilder result = new StringBuilder();
+				result.append("Possible frozen test case\n");
+				ThreadMXBean threadStuff = ManagementFactory.getThreadMXBean();
+				ThreadInfo[] allThreads = threadStuff.getThreadInfo(threadStuff.getAllThreadIds(), 200);
+				for (ThreadInfo threadInfo : allThreads) {
+					result.append("\"");
+					result.append(threadInfo.getThreadName());
+					result.append("\": ");
+					result.append(threadInfo.getThreadState());
+					result.append("\n");
+					final StackTraceElement[] elements = threadInfo.getStackTrace();
+					for (StackTraceElement element : elements) {
+						result.append("    ");
+						result.append(element);
+						result.append("\n");
+					}
+					result.append("\n");
+				}
+				System.out.println(result.toString());
+			}
+		});
+		monitorJob.schedule(millis);
+	}
+
+	public static void done() {
+		if (monitorJob != null) {
+			monitorJob.cancel();
+			monitorJob = null;
+		}
+	}
+}
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/statushandlers/StatusDialogManagerTest.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/statushandlers/StatusDialogManagerTest.java
index 9f133da9c9c..26653c85a4f 100644
--- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/statushandlers/StatusDialogManagerTest.java
+++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/statushandlers/StatusDialogManagerTest.java
@@ -12,6 +12,10 @@
 
 package org.eclipse.ui.tests.statushandlers;
 
+import java.util.concurrent.Semaphore;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.locks.ReentrantLock;
+
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.MultiStatus;
@@ -34,6 +38,7 @@ import org.eclipse.swt.layout.GridData;
 import org.eclipse.swt.widgets.Button;
 import org.eclipse.swt.widgets.Composite;
 import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Display;
 import org.eclipse.swt.widgets.Event;
 import org.eclipse.swt.widgets.Label;
 import org.eclipse.swt.widgets.Link;
@@ -49,8 +54,10 @@ import org.eclipse.ui.statushandlers.AbstractStatusAreaProvider;
 import org.eclipse.ui.statushandlers.IStatusAdapterConstants;
 import org.eclipse.ui.statushandlers.StatusAdapter;
 import org.eclipse.ui.statushandlers.StatusManager;
+import org.eclipse.ui.statushandlers.StatusManager.INotificationListener;
 import org.eclipse.ui.statushandlers.WorkbenchErrorHandler;
 import org.eclipse.ui.statushandlers.WorkbenchStatusDialogManager;
+import org.eclipse.ui.tests.concurrency.FreezeMonitor;
 import org.eclipse.ui.tests.harness.util.UITestCase;
 
 import junit.framework.TestCase;
@@ -73,11 +80,12 @@ public class StatusDialogManagerTest extends TestCase {
 
 	@Override
 	protected void setUp() throws Exception {
+		super.setUp();
 		automatedMode = ErrorDialog.AUTOMATED_MODE;
 		wsdm = new WorkbenchStatusDialogManager(null, null);
 		ErrorDialog.AUTOMATED_MODE = false;
 		wsdm.setProperty(IStatusDialogConstants.ANIMATION, Boolean.FALSE);
-		super.setUp();
+		FreezeMonitor.expectCompletionIn(60_000);
 	}
 
 	public void testBlockingAppearance() {
@@ -1049,6 +1057,100 @@ public class StatusDialogManagerTest extends TestCase {
 		assertTrue("Custom support area provider should be consulted", consulted[0]);
 	}
 
+	public void testDeadlockFromBug501681() throws Exception {
+		assertNotNull("Test must run in UI thread", Display.getCurrent());
+
+		final StatusAdapter statusAdapter = createStatusAdapter("Oops");
+		AtomicReference<StatusAdapter[]> reported = new AtomicReference<>();
+		INotificationListener listener = (type, adapters) -> {
+			reported.set(adapters);
+		};
+
+		AtomicReference<ReentrantLock> lock = new AtomicReference<>();
+		lock.set(new ReentrantLock());
+		final Semaphore semaphore = new Semaphore(0);
+
+		// This simulates a thread locked some shared resource
+		// It will release lock only if "wait" is set to "false"
+		Thread t = new Thread(() -> {
+			lock.get().lock();
+			try {
+				semaphore.acquire();
+			} catch (InterruptedException e) {
+				// continue
+			}
+			lock.get().unlock();
+		});
+		t.start();
+
+		// Wait for thread to acquire the lock
+		while (!lock.get().isLocked()) {
+			Thread.sleep(50);
+		}
+		assertTrue(lock.get().isLocked());
+
+		// Verify status handling works (without blocking)
+		StatusManager.getManager().addListener(listener);
+		StatusManager.getManager().handle(statusAdapter, StatusManager.SHOW | StatusManager.LOG);
+		assertSame(statusAdapter, reported.get()[0]);
+		Shell shell = StatusDialogUtil.getStatusShell();
+		if (shell != null) {
+			shell.dispose();
+		}
+		reported.set(null);
+
+		// Verify status handling works with blocking
+		final StatusAdapter statusAdapter2 = createStatusAdapter("Oops2");
+		try {
+			// this job will try to report some "blocking" status
+			// if it does NOT deadlock, the "wait" will be unset and lock will
+			// be released
+			Job badJob = new Job("Will report blocking error") {
+				@Override
+				protected IStatus run(IProgressMonitor monitor) {
+					StatusManager.getManager().handle(statusAdapter2, StatusManager.BLOCK | StatusManager.LOG);
+					// stop blocking thread
+					semaphore.release();
+					return Status.OK_STATUS;
+				}
+			};
+
+			// start later so that we have time to lock ourselves
+			badJob.schedule(100);
+
+			// This will now block UI until the lock is released
+			// Without the patch for bug 501681 this will never happen
+			// because the job we start in a moment will wait for UI thread to
+			// show the blocking dialog
+			lock.get().lock();
+
+			// make sure job was done
+			badJob.join();
+		} finally {
+			// Will "unblock" the blocking dialog shown by the job, must be
+			// posted with delay, because dialog is not yet shown
+			postUnblockingTask();
+
+			// Allow the blocking dialog to be shown
+			UITestCase.processEvents();
+			StatusManager.getManager().removeListener(listener);
+		}
+		assertFalse("Job should successfully finish", semaphore.hasQueuedThreads());
+		assertNotNull("Status adapter was not logged", reported.get());
+		assertSame(statusAdapter2, reported.get()[0]);
+	}
+
+	private void postUnblockingTask() {
+		Display.getCurrent().timerExec(100, () -> {
+			Shell shell = StatusDialogUtil.getStatusShellImmediately();
+			if (shell != null) {
+				shell.dispose();
+			} else {
+				// shell not shown yet? re-post
+				postUnblockingTask();
+			}
+		});
+	}
 
 	/**
 	 * Delivers custom support area.
@@ -1238,6 +1340,7 @@ public class StatusDialogManagerTest extends TestCase {
 	@Override
 	protected void tearDown() throws Exception {
 		Shell shell = StatusDialogUtil.getStatusShell();
+		FreezeMonitor.done();
 		if (shell != null) {
 			shell.dispose();
 			WorkbenchStatusDialogManagerImpl impl = (WorkbenchStatusDialogManagerImpl) wsdm
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/statushandlers/StatusDialogUtil.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/statushandlers/StatusDialogUtil.java
index ff82c48d981..96fa6147289 100644
--- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/statushandlers/StatusDialogUtil.java
+++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/statushandlers/StatusDialogUtil.java
@@ -59,6 +59,10 @@ public class StatusDialogUtil {
 
 	public static Shell getStatusShell(){
 		UITestCase.waitForJobs(100, 1000);
+		return getStatusShellImmediately();
+	}
+
+	public static Shell getStatusShellImmediately() {
 		Shell[] shells = Display.getDefault().getShells();
 		for (int i = 0; i < shells.length; i++) {
 			if (shells[i].getText().equals("Problem Occurred")
diff --git a/tests/org.eclipse.ui.tests/META-INF/MANIFEST.MF b/tests/org.eclipse.ui.tests/META-INF/MANIFEST.MF
index 5e73ce0ae80..649e040c128 100644
--- a/tests/org.eclipse.ui.tests/META-INF/MANIFEST.MF
+++ b/tests/org.eclipse.ui.tests/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: Eclipse UI Tests
 Bundle-SymbolicName: org.eclipse.ui.tests; singleton:=true
-Bundle-Version: 3.11.2.qualifier
+Bundle-Version: 3.12.0.qualifier
 Eclipse-BundleShape: dir
 Bundle-Activator: org.eclipse.ui.tests.TestPlugin
 Bundle-Vendor: Eclipse.org
diff --git a/tests/org.eclipse.ui.tests/pom.xml b/tests/org.eclipse.ui.tests/pom.xml
index 2637ef9ca35..448e4280cd6 100644
--- a/tests/org.eclipse.ui.tests/pom.xml
+++ b/tests/org.eclipse.ui.tests/pom.xml
@@ -19,7 +19,7 @@
   </parent>
   <groupId>org.eclipse.ui</groupId>
   <artifactId>org.eclipse.ui.tests</artifactId>
-  <version>3.11.2-SNAPSHOT</version>
+  <version>3.12.0-SNAPSHOT</version>
   <packaging>eclipse-test-plugin</packaging>
 
   <properties>