Bug 421303 - Potential deadlock in IOConsolePartitioner.streamAppended

The deadlock requires that a separate thread is write large amounts of
content rapidly to console and while doing this holds lock(s) with UI
thread may require.

An example is that both (UI and non-UI thread) write to console using
the same IOConsoleOutputStream and therefore sharing the streams write
lock.

Change-Id: I5c72b60e765ec550928ba872f089494593475856
Signed-off-by: Paul Pazderski <paul-eclipse@ppazderski.de>

2 files changed, 71 insertions, 3 deletions

diff --git a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/IOConsoleTests.java b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/IOConsoleTests.java
index 1277b8196..d70fe9ff6 100644
--- a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/IOConsoleTests.java
+++ b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/IOConsoleTests.java
@@ -28,8 +28,11 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.eclipse.core.runtime.ILogListener;
+import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Platform;
+import org.eclipse.core.runtime.Status;
+import org.eclipse.core.runtime.jobs.Job;
 import org.eclipse.debug.tests.AbstractDebugTest;
 import org.eclipse.debug.tests.TestUtil;
 import org.eclipse.debug.tests.TestsPlugin;
@@ -750,6 +753,67 @@ public class IOConsoleTests extends AbstractDebugTest {
 	}
 
 	/**
+	 * Regression test for deadlock in stream processing.
+	 */
+	public void testBug421303_StreamProcessingDeadlock() throws Exception {
+		// Test situation is that UI thread and another thread both write a
+		// large amount of output into same IOConsoleOutputStream at same time
+		// where the non-UI thread is writing first.
+		// Test includes a watchdog thread which will break the deadlock (if
+		// happened) so the test can end in a reasonable amount of time.
+		final IOConsoleTestUtil c = getTestUtil("Test Bug 421303 Stream processing deadlock");
+		final String veryLongString = String.join("", Collections.nCopies(20000, "0123456789"));
+		final Exception[] jobException = new Exception[1];
+		final AtomicBoolean deadlocked = new AtomicBoolean(false);
+		Job job = new Job("Async out") {
+			@Override
+			protected IStatus run(IProgressMonitor monitor) {
+				synchronized (c) {
+					c.notifyAll();
+				}
+				try {
+					c.writeFast(veryLongString);
+				} catch (IOException e) {
+					jobException[0] = e;
+				}
+				return Status.OK_STATUS;
+			}
+		};
+		Thread watchdog = new Thread(() -> {
+			try {
+				Thread.sleep(testTimeout);
+				synchronized (c) {
+					c.notifyAll();
+				}
+				if (job.getThread() != null && job.getThread().isAlive()) {
+					deadlocked.set(true);
+					job.getThread().interrupt();
+				}
+			} catch (InterruptedException e) {
+			}
+		}, "Watchdog");
+		watchdog.setDaemon(true);
+		watchdog.start();
+
+		synchronized (c) {
+			job.schedule();
+			c.wait();
+		}
+		// ensure other thread is writing first
+		Thread.yield();
+		Thread.sleep(50);
+		c.writeFast(veryLongString);
+
+		watchdog.interrupt();
+		watchdog.join(1000);
+		if (jobException[0] != null) {
+			throw jobException[0];
+		}
+		assertFalse("Deadlock in stream processing.", deadlocked.get());
+		closeConsole(c);
+	}
+
+	/**
 	 * Check if there is any offset which received two styles.
 	 *
 	 * @param styles the styles to check
diff --git a/org.eclipse.ui.console/src/org/eclipse/ui/internal/console/IOConsolePartitioner.java b/org.eclipse.ui.console/src/org/eclipse/ui/internal/console/IOConsolePartitioner.java
index f6df185ee..8e9769cf2 100644
--- a/org.eclipse.ui.console/src/org/eclipse/ui/internal/console/IOConsolePartitioner.java
+++ b/org.eclipse.ui.console/src/org/eclipse/ui/internal/console/IOConsolePartitioner.java
@@ -669,7 +669,7 @@ public class IOConsolePartitioner
 	 * A stream has been appended, add to pendingPartions list and schedule
 	 * updateJob. updateJob is scheduled with a slight delay, this allows the
 	 * console to run the job less frequently and update the document with a greater
-	 * amount of data each time the job is run
+	 * amount of data each time the job is run.
 	 *
 	 * @param stream The stream that was written to.
 	 * @param s      The string that should be appended to the document.
@@ -701,11 +701,15 @@ public class IOConsolePartitioner
 			if (pendingSize > 160000) {
 				if (Display.getCurrent() == null) {
 					try {
-						pendingPartitions.wait();
+						// Block thread to give UI time to process pending output.
+						// Do not wait forever. Current thread and UI thread might share locks. An
+						// example is bug 421303 where current thread and UI thread both write to
+						// console and therefore both need the write lock for IOConsoleOutputStream.
+						pendingPartitions.wait(1000);
 					} catch (InterruptedException e) {
 					}
 				} else {
-					// if we are in UI thread we cannot lock it, so process queued output.
+					// If we are in UI thread we cannot lock it, so process queued output.
 					processPendingPartitions();
 				}
 			}