Bug 505920 - JobManager.beginRule leaks the interrupted state
Remove the temporary workaround, now that the buggy tests appear to have
been fixed.
Change-Id: Ibec382badbe0dfe59d013385212c0ac9a5e7f155
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 828b5c0..4015b9c 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 @@
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 2f8e4ff..5f87980 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 @@
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 @@
// 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 e2e47e7..bd84632 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.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 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;
- }
- // 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();
- return Status.OK_STATUS;
- }
- };
- yieldA.setRule(rule);
- yieldA.schedule();
-
- yieldB.setRule(rule);
- yieldB.schedule();
-
- // Step 1
- semaphoreA.release();
-
- // Block until step 4 and step 5 have occurred
- mainThreadSemaphore.acquire(2);
- if (failureMessage[0] != null) {
- assertNull(failureMessage[0], failureMessage[0]);
+ // 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) {
+ }
- assertTrue("yieldRule should have thrown OperationCanceledException", operationWasCanceled[0]);
- } finally {
- JobManager.reactToInterruption = oldReactToInterruption;
+ } 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();
+ return Status.OK_STATUS;
+ }
+ };
+ yieldA.setRule(rule);
+ yieldA.schedule();
+
+ yieldB.setRule(rule);
+ yieldB.schedule();
+
+ // Step 1
+ semaphoreA.release();
+
+ // 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() {
