Bug 505920 - JobManager.beginRule leaks the interrupted stateI20170226-2000I20170225-2000I20170224-2000I20170223-2000

Remove the temporary workaround, now that the buggy tests appear to have
been fixed.

Change-Id: Ibec382badbe0dfe59d013385212c0ac9a5e7f155

3 files changed, 67 insertions, 107 deletions

diff --git a/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/JobManager.java b/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/JobManager.java
index 828b5c00b..4015b9c96 100644
--- a/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/JobManager.java
+++ b/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/JobManager.java
@@ -92,22 +92,6 @@ public class JobManager implements IJobManager, DebugOptionsListener {
 	private static JobManager instance;
 
 	/**
-	 * Set this to true if the Job framework should react to interruption by
-	 * throwing OperationCanceledException. Set this to false if it should
-	 * ignore thread interruption that was definitely not triggered by the Jobs
-	 * framework itself.
-	 * <p>
-	 * Although setting this to true is the correct behavior, there are
-	 * currently some unit tests that rely on the fact that thread interruption
-	 * is being ignored. Set this to true to help locate and fix these tests.
-	 * Set it to false in production until this is done.
-	 * <p>
-	 * TODO: remove this flag and set it permanently to true once bug 506294 is
-	 * fixed.
-	 */
-	public static boolean reactToInterruption;
-
-	/**
 	 * Scheduling rule used for validation of client-defined rules.
 	 */
 	private static final ISchedulingRule nullRule = new ISchedulingRule() {
diff --git a/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/ThreadJob.java b/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/ThreadJob.java
index 2f8e4ff0c..5f8798075 100644
--- a/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/ThreadJob.java
+++ b/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/ThreadJob.java
@@ -191,13 +191,6 @@ class ThreadJob extends Job {
 		InternalJob blockingJob = manager.findBlockingJob(threadJob);
 		Thread blocker = blockingJob == null ? null : blockingJob.getThread();
 		ThreadJob result;
-		// The reactToInterruption flag allows this method to react correctly
-		// to Thread.interrupt(). However, there are unit tests that currently
-		// rely on the fact that the interrupt flag is ignored. If this flag
-		// is disabled, we don't react if the thread was already interrupted
-		// prior to the start of this method call -- we'll just restore the
-		// interrupted flag to its initial state at the end.
-		boolean setInterruptFlagAtEnd = !JobManager.reactToInterruption && Thread.interrupted();
 		boolean interruptedDuringWaitForRun;
 		try {
 			// just return if lock listener decided to grant immediate access
@@ -212,16 +205,6 @@ class ThreadJob extends Job {
 			// throwing some other exception here.
 			interruptedDuringWaitForRun = Thread.interrupted();
 			manager.getLockManager().aboutToRelease();
-			// If the thread was interrupted prior to the call to joinRun, the
-			// interruption was not caused by the jobs framework. Although the
-			// correct behavior here is to still terminate cleanly and throw
-			// an OperationCanceledException, there are some existing unit tests
-			// that are interrupting threads and are relying on the fact that
-			// nobody reacts to the interruption. To keep these tests working,
-			// we restore the interrupted flag back to the state it had
-			if (setInterruptFlagAtEnd) {
-				Thread.currentThread().interrupt();
-			}
 		}
 
 		// During the call to waitForRun, we use the thread's interrupt flag to
diff --git a/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/YieldTest.java b/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/YieldTest.java
index e2e47e777..bd84632fe 100644
--- a/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/YieldTest.java
+++ b/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/YieldTest.java
@@ -14,7 +14,6 @@ import java.util.*;
 import java.util.concurrent.Semaphore;
 import junit.framework.Test;
 import junit.framework.TestSuite;
-import org.eclipse.core.internal.jobs.JobManager;
 import org.eclipse.core.runtime.*;
 import org.eclipse.core.runtime.jobs.*;
 import org.eclipse.core.tests.harness.TestBarrier;
@@ -861,89 +860,83 @@ public class YieldTest extends AbstractJobManagerTest {
 	}
 
 	public void testYieldIsInterruptable() throws Exception {
-		boolean oldReactToInterruption = JobManager.reactToInterruption;
-		try {
-			JobManager.reactToInterruption = true;
-			Semaphore semaphoreA = new Semaphore(0);
-			Semaphore semaphoreB = new Semaphore(0);
-			Semaphore mainThreadSemaphore = new Semaphore(0);
-			final PathRule rule = new PathRule(getName());
-			String[] failureMessage = new String[1];
-			boolean[] operationWasCanceled = new boolean[1];
-			final Job yieldB = new Job(getName() + " YieldingB") {
-				@Override
-				protected IStatus run(IProgressMonitor monitor) {
-					// Step 2
-					semaphoreA.release();
-					try {
-						// Block until thread yieldA reaches step 3
-						semaphoreB.acquire();
-					} catch (InterruptedException e) {
-						failureMessage[0] = "yieldB was interrupted too early";
-						return Status.OK_STATUS;
-					}
-					// Step 5
-					mainThreadSemaphore.release();
+		Semaphore semaphoreA = new Semaphore(0);
+		Semaphore semaphoreB = new Semaphore(0);
+		Semaphore mainThreadSemaphore = new Semaphore(0);
+		final PathRule rule = new PathRule(getName());
+		String[] failureMessage = new String[1];
+		boolean[] operationWasCanceled = new boolean[1];
+		final Job yieldB = new Job(getName() + " YieldingB") {
+			@Override
+			protected IStatus run(IProgressMonitor monitor) {
+				// Step 2
+				semaphoreA.release();
+				try {
+					// Block until thread yieldA reaches step 3
+					semaphoreB.acquire();
+				} catch (InterruptedException e) {
+					failureMessage[0] = "yieldB was interrupted too early";
 					return Status.OK_STATUS;
 				}
-			};
-			Job yieldA = new Job(getName() + " YieldingA") {
-				@Override
-				protected IStatus run(IProgressMonitor monitor) {
-					try {
-						// Block until the main thread reaches step 1
-						semaphoreA.acquire();
-					} catch (InterruptedException e) {
-						failureMessage[0] = "yieldA was interrupted too early";
-						return Status.OK_STATUS;
+				// Step 5
+				mainThreadSemaphore.release();
+				return Status.OK_STATUS;
+			}
+		};
+		Job yieldA = new Job(getName() + " YieldingA") {
+			@Override
+			protected IStatus run(IProgressMonitor monitor) {
+				try {
+					// Block until the main thread reaches step 1
+					semaphoreA.acquire();
+				} catch (InterruptedException e) {
+					failureMessage[0] = "yieldA was interrupted too early";
+					return Status.OK_STATUS;
+				}
+				// If this fails to interrupt the following yieldRule call,
+				// this thread will deadlock.
+				Thread.currentThread().interrupt();
+				try {
+					// Allow yieldB to start execution
+					while (yieldRule(null) != yieldB) {
 					}
-					// If this fails to interrupt the following yieldRule call,
-					// this thread will deadlock.
-					Thread.currentThread().interrupt();
-					try {
-						// Allow yieldB to start execution
-						while (yieldRule(null) != yieldB) {
-						}
 
-					} catch (OperationCanceledException e) {
-						operationWasCanceled[0] = true;
-					}
-					if (Thread.interrupted()) {
-						failureMessage[0] = "The thread was interrupted after yieldRule returned";
-					}
-					try {
-						// Block until yieldB reaches step 2
-						semaphoreA.acquire();
-					} catch (InterruptedException e) {
-						failureMessage[0] = "yieldA was interrupted too early";
-						return Status.OK_STATUS;
-					}
-					// Step 3
-					semaphoreB.release();
-					// Step 4
-					mainThreadSemaphore.release();
+				} catch (OperationCanceledException e) {
+					operationWasCanceled[0] = true;
+				}
+				if (Thread.interrupted()) {
+					failureMessage[0] = "The thread was interrupted after yieldRule returned";
+				}
+				try {
+					// Block until yieldB reaches step 2
+					semaphoreA.acquire();
+				} catch (InterruptedException e) {
+					failureMessage[0] = "yieldA was interrupted too early";
 					return Status.OK_STATUS;
 				}
-			};
-			yieldA.setRule(rule);
-			yieldA.schedule();
-
-			yieldB.setRule(rule);
-			yieldB.schedule();
+				// Step 3
+				semaphoreB.release();
+				// Step 4
+				mainThreadSemaphore.release();
+				return Status.OK_STATUS;
+			}
+		};
+		yieldA.setRule(rule);
+		yieldA.schedule();
 
-			// Step 1
-			semaphoreA.release();
+		yieldB.setRule(rule);
+		yieldB.schedule();
 
-			// Block until step 4 and step 5 have occurred
-			mainThreadSemaphore.acquire(2);
-			if (failureMessage[0] != null) {
-				assertNull(failureMessage[0], failureMessage[0]);
-			}
+		// Step 1
+		semaphoreA.release();
 
-			assertTrue("yieldRule should have thrown OperationCanceledException", operationWasCanceled[0]);
-		} finally {
-			JobManager.reactToInterruption = oldReactToInterruption;
+		// Block until step 4 and step 5 have occurred
+		mainThreadSemaphore.acquire(2);
+		if (failureMessage[0] != null) {
+			assertNull(failureMessage[0], failureMessage[0]);
 		}
+
+		assertTrue("yieldRule should have thrown OperationCanceledException", operationWasCanceled[0]);
 	}
 
 	public void testYieldJobToJobsInterleaved() {