Bug 365770 - Race condition in IOConsolePartitioner causes consoleY20200120-0020I20200120-0530I20200120-0445I20200120-0355I20200120-0115I20200119-2330I20200119-1800I20200118-1800I20200117-1800

output to be lost or not cleared

Due to the race condition when calling

  outStream.print("First");
  console.clearConsole();
  outStream.print("Second");

from a single thread the actual console content could be "FirstSecond"
or "" where only "Second" is correct.

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

3 files changed, 95 insertions, 46 deletions

diff --git a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/IOConsoleTestUtil.java b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/IOConsoleTestUtil.java
index 9f29c7df0..865561f82 100644
--- a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/IOConsoleTestUtil.java
+++ b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/IOConsoleTestUtil.java
@@ -505,6 +505,17 @@ public final class IOConsoleTestUtil {
 	}
 
 	/**
+	 * Select all content in console.
+	 *
+	 * @return this {@link IOConsoleTestUtil} to chain methods
+	 * @see StyledText#setSelection(int, int)
+	 */
+	public IOConsoleTestUtil selectAll() {
+		textPanel.selectAll();
+		return this;
+	}
+
+	/**
 	 * Check if console content equals the expected content.
 	 *
 	 * @param expectedContent content expect in console
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 d70fe9ff6..bd18bd8ba 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
@@ -210,6 +210,23 @@ public class IOConsoleTests extends AbstractDebugTest {
 		c.clear().insertTypingAndVerify("I").write("O").verifyContent("IO").verifyPartitions();
 		c.insert("\r\n").clear();
 
+		c.insertTypingAndVerify("some user input").selectAll().backspace();
+		c.verifyContent("").verifyPartitions();
+
+		// test (almost) simultaneous write and clear
+		c.writeFast("to be removed").clear().verifyPartitions();
+		// Do not use clear() from test util here. Test requires an immediate
+		// write after clear. The util's clear() method blocks until console is
+		// actually cleared.
+		c.getConsole().clearConsole();
+		c.writeAndVerify("do not remove this").verifyPartitions().clear();
+		final String longString = String.join("", Collections.nCopies(1000, "012345678\n"));
+		c.getConsole().clearConsole();
+		c.writeAndVerify(longString).verifyPartitions().clear();
+		final String veryLongString = String.join("", Collections.nCopies(20000, "abcdefghi\n"));
+		c.getConsole().clearConsole();
+		c.writeAndVerify(veryLongString).verifyPartitions().clear();
+
 		closeConsole(c, "i", "I");
 	}
 
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 8e9769cf2..912e008ad 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
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2019 IBM Corporation and others.
+ * Copyright (c) 2000, 2020 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -16,6 +16,7 @@
  *                          Bug 550618: getStyleRanges produced invalid overlapping styles
  *                          Bug 550621: Implementation of IConsoleDocumentPartitionerExtension
  *                          Bug 76936:  Support interpretation of \b and \r in console output
+ *                          Bug 365770: Race condition in console clearing
  *******************************************************************************/
 package org.eclipse.ui.internal.console;
 
@@ -419,9 +420,9 @@ public class IOConsolePartitioner
 	/**
 	 * Enforces the buffer size.
 	 * <p>
-	 * When the number of lines in the document exceeds the high water mark, the
-	 * beginning of the document is trimmed until the number of lines equals the low
-	 * water mark.
+	 * When the document length exceeds the high water mark, the beginning of the
+	 * document is trimmed until the document length is approximately the low water
+	 * mark.
 	 * </p>
 	 */
 	private void checkBufferSize() {
@@ -429,7 +430,7 @@ public class IOConsolePartitioner
 			int length = document.getLength();
 			if (length > highWaterMark) {
 				if (trimJob.getState() == Job.NONE) { // if the job isn't already running
-					trimJob.setOffset(length - lowWaterMark);
+					trimJob.setTrimLineOffset(length - lowWaterMark);
 					trimJob.schedule();
 				}
 			}
@@ -440,8 +441,14 @@ public class IOConsolePartitioner
 	 * Clears the console content.
 	 */
 	public void clearBuffer() {
-		trimJob.setOffset(-1);
-		trimJob.schedule();
+		synchronized (pendingPartitions) {
+			pendingPartitions.clear();
+			pendingSize = 0;
+		}
+		synchronized (partitions) {
+			trimJob.setTrimOffset(document.getLength());
+			trimJob.schedule();
+		}
 	}
 
 	@Override
@@ -1083,10 +1090,14 @@ public class IOConsolePartitioner
 	 */
 	private class TrimJob extends WorkbenchJob {
 
+		/** Trims output up to given offset. */
+		private int truncateOffset;
+
 		/**
-		 * Trims output up to the line containing the given offset, or all output if -1.
+		 * If <code>true</code> trim only to start of line containing the
+		 * {@link #truncateOffset}.
 		 */
-		private int truncateOffset;
+		private boolean truncateToOffsetLineStart;
 
 		/**
 		 * Creates a new job to trim the buffer.
@@ -1099,10 +1110,21 @@ public class IOConsolePartitioner
 		/**
 		 * Sets the trim offset.
 		 *
-		 * @param offset trims output up to the line containing the given offset
+		 * @param offset trims console content up to this offset
 		 */
-		public void setOffset(int offset) {
+		public void setTrimOffset(int offset) {
 			truncateOffset = offset;
+			truncateToOffsetLineStart = false;
+		}
+
+		/**
+		 * Sets the trim offset.
+		 *
+		 * @param offset trims output up to the line containing this offset
+		 */
+		public void setTrimLineOffset(int offset) {
+			truncateOffset = offset;
+			truncateToOffsetLineStart = true;
 		}
 
 		@Override
@@ -1112,46 +1134,45 @@ public class IOConsolePartitioner
 					return Status.OK_STATUS;
 				}
 
-				int length = document.getLength();
-				if (truncateOffset < length) {
-					try {
-						if (truncateOffset < 0) {
-							// clear
-							updateType = DocUpdateType.TRIM;
-							document.set(""); //$NON-NLS-1$
-						} else {
-							// overflow
-							int cutoffLine = document.getLineOfOffset(truncateOffset);
-							int cutOffset = document.getLineOffset(cutoffLine);
-
-							// set the new length of the first partition
-							IOConsolePartition partition = (IOConsolePartition) getPartition(cutOffset);
-							partition.setLength(partition.getOffset() + partition.getLength() - cutOffset);
-
-							updateType = DocUpdateType.TRIM;
-							document.replace(0, cutOffset, ""); //$NON-NLS-1$
-
-							// remove partitions and reset Partition offsets
-							int index = partitions.indexOf(partition);
-							for (int i = 0; i < index; i++) {
-								partitions.remove(0);
-							}
+				try {
+					int length = document.getLength();
+					int cutOffset = truncateOffset;
+					if (truncateToOffsetLineStart) {
+						int cutoffLine = document.getLineOfOffset(truncateOffset);
+						cutOffset = document.getLineOffset(cutoffLine);
+					}
+					if (cutOffset >= length) {
+						updateType = DocUpdateType.TRIM;
+						document.set(""); //$NON-NLS-1$
+					} else {
+						// set the new length of the first partition
+						IOConsolePartition partition = getIOPartition(cutOffset);
+						partition.setLength(partition.getOffset() + partition.getLength() - cutOffset);
 
-							int offset = 0;
-							for (IOConsolePartition p : partitions) {
-								p.setOffset(offset);
-								offset += p.getLength();
-							}
+						updateType = DocUpdateType.TRIM;
+						document.replace(0, cutOffset, ""); //$NON-NLS-1$
 
-							// fix output offset
-							int removedLength = cutOffset;
-							outputOffset = Math.max(outputOffset - removedLength, 0);
+						// remove partitions and reset Partition offsets
+						int index = partitions.indexOf(partition);
+						for (int i = 0; i < index; i++) {
+							partitions.remove(0);
 						}
-						if (ASSERT) {
-							checkPartitions();
+
+						int offset = 0;
+						for (IOConsolePartition p : partitions) {
+							p.setOffset(offset);
+							offset += p.getLength();
 						}
-					} catch (BadLocationException e) {
+
+						// fix output offset
+						int removedLength = cutOffset;
+						outputOffset = Math.max(outputOffset - removedLength, 0);
+					}
+					if (ASSERT) {
+						checkPartitions();
 					}
+				} catch (BadLocationException e) {
+					log(e);
 				}
 			}
 			return Status.OK_STATUS;