diff options
3 files changed, 260 insertions, 62 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 b42fe112d..e93f5b5b6 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 @@ -90,6 +90,23 @@ public class JobManager implements IJobManager, DebugOptionsListener { * of updating it. */ 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. */ 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 d7f0a9110..2f8e4ff0c 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 @@ -190,14 +190,47 @@ class ThreadJob extends Job { // check if there is a blocking thread before waiting 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 if (manager.getLockManager().aboutToWait(blocker)) return threadJob; - return waitForRun(threadJob, monitor, blockingJob, blocker); + result = waitForRun(threadJob, monitor, blockingJob, blocker); } finally { + // We need to check for interruption unconditionally in order to + // ensure we clear the thread's interrupted state. However, we only + // throw an OperationCanceledException outside of the finally block + // because we only want to throw that exception if we're not already + // 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 + // trigger cancellation, so thread interruption at this time should + // trigger an OCE. + if (interruptedDuringWaitForRun) { + throw new OperationCanceledException(); } + return result; } private static ThreadJob waitForRun(ThreadJob threadJob, IProgressMonitor monitor, InternalJob blockingJob, @@ -231,7 +264,7 @@ class ThreadJob extends Job { // 4) Monitor is canceled. while (true) { // monitor is foreign code so do not hold locks while calling into monitor - if (isCanceled(monitor)) + if (interrupted || isCanceled(monitor)) // Condition #4. throw new OperationCanceledException(); // Try to run the job. If result is null, this job was allowed to run. @@ -287,17 +320,16 @@ class ThreadJob extends Job { manager.getLockManager().removeLockWaitThread(currentThread, threadJob.getRule()); } } finally { - if (interrupted) - Thread.currentThread().interrupt(); //only update the lock state if we ended up using the thread job that was given to us waitEnd(threadJob, threadJob == result, monitor); if (threadJob == result) { if (waiting) manager.implicitJobs.removeWaiting(threadJob); } - if (canBlock) + if (canBlock) { // must unregister monitoring this job manager.endMonitoring(threadJob); + } } } diff --git a/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/IJobManagerTest.java b/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/IJobManagerTest.java index 71d6c4053..0ea8eada0 100644 --- a/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/IJobManagerTest.java +++ b/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/IJobManagerTest.java @@ -11,6 +11,7 @@ package org.eclipse.core.tests.runtime.jobs; import java.util.*; +import java.util.concurrent.Semaphore; import junit.framework.*; import org.eclipse.core.runtime.*; import org.eclipse.core.runtime.jobs.*; @@ -84,16 +85,18 @@ public class IJobManagerTest extends AbstractJobManagerTest { */ public void assertState(String msg, Job job, int expectedState) { int actualState = job.getState(); - if (actualState != expectedState) + if (actualState != expectedState) { assertTrue(msg + ": expected state: " + printState(expectedState) + " actual state: " + printState(actualState), false); + } } /** * Cancels a list of jobs */ protected void cancel(ArrayList<Job> jobs) { - for (Iterator<Job> it = jobs.iterator(); it.hasNext();) + for (Iterator<Job> it = jobs.iterator(); it.hasNext();) { it.next().cancel(); + } } private String printState(int state) { @@ -124,9 +127,11 @@ public class IJobManagerTest extends AbstractJobManagerTest { @Override protected void tearDown() throws Exception { - for (int i = 0; i < jobListeners.length; i++) - if (jobListeners[i] instanceof TestJobListener) + for (int i = 0; i < jobListeners.length; i++) { + if (jobListeners[i] instanceof TestJobListener) { ((TestJobListener) jobListeners[i]).cancelAllJobs(); + } + } waitForCompletion(); for (int i = 0; i < jobListeners.length; i++) { manager.removeJobChangeListener(jobListeners[i]); @@ -212,6 +217,104 @@ public class IJobManagerTest extends AbstractJobManagerTest { } /** + * Tests that if we call beginRule with a monitor that has already been + * cancelled, it won't try to obtain the rule. + */ + public void testCancellationPriorToBeginRuleWontHoldRule() throws Exception { + final Semaphore mainThreadSemaphore = new Semaphore(0); + final Semaphore lockSemaphore = new Semaphore(0); + final PathRule rule = new PathRule("testBeginRuleNoEnd"); + IProgressMonitor cancelledMonitor = SubMonitor.convert(null); + cancelledMonitor.setCanceled(true); + + // Create a job that will hold the lock until the semaphore is signalled + Job job = Job.create("", monitor -> { + mainThreadSemaphore.release(); + try { + lockSemaphore.acquire(); + } catch (InterruptedException e) { + } + }); + job.setRule(rule); + job.schedule(); + + // Block until the job acquires the lock + mainThreadSemaphore.acquire(); + boolean canceledExceptionThrown = false; + try { + // This will deadlock if it attempts to acquire the rule, and will + // throw an OCE without doing anything if it is working correctly. + manager.beginRule(rule, cancelledMonitor); + } catch (OperationCanceledException e) { + canceledExceptionThrown = true; + } finally { + // Code which follows the recommended pattern documented in + // beginRule will call endRule even if beginRule threw an OCE. + // Verify that calling endRule in this situation won't throw any + // exceptions. + manager.endRule(rule); + } + lockSemaphore.release(); + boolean interrupted = Thread.interrupted(); + assertTrue("An OperationCancelledException should have been thrown", canceledExceptionThrown); + assertFalse("The Thread.interrupted() state leaked", interrupted); + } + + /** + * Tests that if our monitor is cancelled while we're waiting on beginRule, + * it will stop waiting, will throw an {@link OperationCanceledException}, + * and will clear the Thread.interrupted() flag. + */ + public void testCancellationWhileWaitingOnRule() throws Exception { + final Semaphore mainThreadSemaphore = new Semaphore(0); + final Semaphore lockSemaphore = new Semaphore(0); + final PathRule rule = new PathRule("testBeginRuleNoEnd"); + final NullProgressMonitor rootMonitor = new NullProgressMonitor(); + // We use a SubMonitor here to work around a special case in the + // JobManager code that ignores NullProgressMonitor. + IProgressMonitor nestedMonitor = SubMonitor.convert(rootMonitor); + nestedMonitor.setCanceled(false); + + // Create a job that will hold the lock until the semaphore is signalled + Job job = Job.create("", monitor -> { + mainThreadSemaphore.release(); + try { + lockSemaphore.acquire(); + } catch (InterruptedException e) { + } + }); + job.setRule(rule); + job.schedule(); + + // Block until the job acquires the lock + mainThreadSemaphore.acquire(); + + // Create a job that will cancel our monitor in 100ms + Job cancellationJob = Job.create("", monitor -> { + rootMonitor.setCanceled(true); + }); + cancellationJob.schedule(100); + + boolean canceledExceptionThrown = false; + // Now try to obtain the rule that is currently held by "job". + try { + manager.beginRule(rule, nestedMonitor); + } catch (OperationCanceledException e) { + canceledExceptionThrown = true; + } finally { + // Code which follows the recommended pattern documented in + // beginRule will call endRule even if beginRule threw an OCE. + // Verify that calling endRule in this situation won't throw any + // exceptions. + manager.endRule(rule); + } + lockSemaphore.release(); + boolean interrupted = Thread.interrupted(); + assertTrue("An OperationCancelledException should have been thrown", canceledExceptionThrown); + assertFalse("The THread.interrupted() state leaked", interrupted); + } + + /** * Tests running a job that begins a rule but never ends it */ public void testBeginRuleNoEnd() { @@ -356,16 +459,18 @@ public class IJobManagerTest extends AbstractJobManagerTest { }; sequenceJob.schedule(); waitForCompletion(sequenceJob); - if (!errors.isEmpty()) + if (!errors.isEmpty()) { throw errors.iterator().next(); + } //now test in a job that has a scheduling rule ISchedulingRule jobRule = new PathRule("/testCurrentRule"); sequenceJob.setRule(jobRule); sequenceJob.schedule(); waitForCompletion(sequenceJob); - if (!errors.isEmpty()) + if (!errors.isEmpty()) { throw errors.iterator().next(); + } } @@ -373,8 +478,9 @@ public class IJobManagerTest extends AbstractJobManagerTest { * Helper method for testing {@link IJobManager#currentRule()}. */ protected void runRuleSequence() { - if (runRuleSequenceInJobWithRule()) + if (runRuleSequenceInJobWithRule()) { return; + } ISchedulingRule parent = new PathRule("/testCurrentRule/parent"); ISchedulingRule child = new PathRule("/testCurrentRule/parent/child"); assertNull(manager.currentRule()); @@ -406,11 +512,13 @@ public class IJobManagerTest extends AbstractJobManagerTest { */ private boolean runRuleSequenceInJobWithRule() { Job currentJob = manager.currentJob(); - if (currentJob == null) + if (currentJob == null) { return false; + } ISchedulingRule jobRule = currentJob.getRule(); - if (jobRule == null) + if (jobRule == null) { return false; + } //we are in a job with a rule, so now run our rule sequence ISchedulingRule parent = new PathRule("/testCurrentRule/parent"); ISchedulingRule child = new PathRule("/testCurrentRule/parent/child"); @@ -449,8 +557,9 @@ public class IJobManagerTest extends AbstractJobManagerTest { long duration = System.currentTimeMillis() - start; assertTrue("1.2: duration: " + duration + " sleep: " + sleepTimes[i], duration >= sleepTimes[i]); //a no-op job shouldn't take any real time - if (PEDANTIC) + if (PEDANTIC) { assertTrue("1.3: duration: " + duration + " sleep: " + sleepTimes[i], duration < sleepTimes[i] + 1000); + } } } @@ -466,11 +575,12 @@ public class IJobManagerTest extends AbstractJobManagerTest { for (int i = 0; i < NUM_JOBS; i++) { //assign half the jobs to the first family, the other half to the second family - if (i % 2 == 0) + if (i % 2 == 0) { jobs[i] = new FamilyTestJob("TestFirstFamily", 1000000, 10, TestJobFamily.TYPE_ONE); - else + } else { /*if(i%2 == 1)*/ jobs[i] = new FamilyTestJob("TestSecondFamily", 1000000, 10, TestJobFamily.TYPE_TWO); + } jobs[i].setRule(rule); jobs[i].schedule(); } @@ -546,17 +656,18 @@ public class IJobManagerTest extends AbstractJobManagerTest { for (int i = 0; i < NUM_JOBS; i++) { //assign four jobs to each family - if (i % 5 == 0) + if (i % 5 == 0) { jobs[i] = new FamilyTestJob("TestFirstFamily", 1000000, 10, TestJobFamily.TYPE_ONE); - else if (i % 5 == 1) + } else if (i % 5 == 1) { jobs[i] = new FamilyTestJob("TestSecondFamily", 1000000, 10, TestJobFamily.TYPE_TWO); - else if (i % 5 == 2) + } else if (i % 5 == 2) { jobs[i] = new FamilyTestJob("TestThirdFamily", 1000000, 10, TestJobFamily.TYPE_THREE); - else if (i % 5 == 3) + } else if (i % 5 == 3) { jobs[i] = new FamilyTestJob("TestFourthFamily", 1000000, 10, TestJobFamily.TYPE_FOUR); - else + } else { /*if(i%5 == 4)*/ jobs[i] = new FamilyTestJob("TestFifthFamily", 1000000, 10, TestJobFamily.TYPE_FIVE); + } jobs[i].setRule(rule); jobs[i].schedule(); @@ -573,8 +684,9 @@ public class IJobManagerTest extends AbstractJobManagerTest { assertTrue("1.0", result.length >= NUM_JOBS); for (int i = 0; i < result.length; i++) { //only test jobs that we know about - if (allJobs.remove(result[i])) + if (allJobs.remove(result[i])) { assertTrue("1." + i, (result[i].belongsTo(first) || result[i].belongsTo(second) || result[i].belongsTo(third) || result[i].belongsTo(fourth) || result[i].belongsTo(fifth))); + } } assertEquals("1.2", 0, allJobs.size()); @@ -673,8 +785,9 @@ public class IJobManagerTest extends AbstractJobManagerTest { assertTrue("11.0", result.length >= 8); for (int i = 0; i < result.length; i++) { //only test jobs that we know about - if (allJobs.remove(result[i])) + if (allJobs.remove(result[i])) { assertTrue("11." + (i + 1), (result[i].belongsTo(third) || result[i].belongsTo(fifth))); + } } assertEquals("11.2", 12, allJobs.size()); @@ -700,8 +813,9 @@ public class IJobManagerTest extends AbstractJobManagerTest { for (int i = 0; i < result.length; i++) { //test jobs that we know about should not be found (they should have all been removed) - if (allJobs.remove(result[i])) + if (allJobs.remove(result[i])) { assertTrue("14." + i, false); + } } assertEquals("15.0", NUM_JOBS, allJobs.size()); allJobs.clear(); @@ -1080,8 +1194,9 @@ public class IJobManagerTest extends AbstractJobManagerTest { assertTrue("2.1", endTime > startTime); //the join call should take no actual time (join call should not block thread at all) - if (PEDANTIC) + if (PEDANTIC) { assertTrue("2.2 start time: " + startTime + " end time: " + endTime, (endTime - startTime) < 300); + } //cancel all jobs manager.cancel(first); @@ -1110,14 +1225,16 @@ public class IJobManagerTest extends AbstractJobManagerTest { final IJobChangeListener listener = new JobChangeAdapter() { @Override public void done(IJobChangeEvent event) { - if (event.getJob() == running) + if (event.getJob() == running) { barrier.waitForStatus(TestBarrier.STATUS_WAIT_FOR_DONE); + } } @Override public void running(IJobChangeEvent event) { - if (event.getJob() == running) + if (event.getJob() == running) { barrier.setStatus(TestBarrier.STATUS_RUNNING); + } } }; Job job = new Job("main job") { @@ -1195,14 +1312,16 @@ public class IJobManagerTest extends AbstractJobManagerTest { final IJobChangeListener listener = new JobChangeAdapter() { @Override public void done(IJobChangeEvent event) { - if (event.getJob() == running) + if (event.getJob() == running) { barrier.waitForStatus(TestBarrier.STATUS_WAIT_FOR_DONE); + } } @Override public void running(IJobChangeEvent event) { - if (event.getJob() == running) + if (event.getJob() == running) { barrier.setStatus(TestBarrier.STATUS_RUNNING); + } } }; Job job = new Job("main job") { @@ -1280,16 +1399,18 @@ public class IJobManagerTest extends AbstractJobManagerTest { final IJobChangeListener listener = new JobChangeAdapter() { @Override public void done(IJobChangeEvent event) { - if (event.getJob() == running) + if (event.getJob() == running) { barrier.waitForStatus(TestBarrier.STATUS_WAIT_FOR_DONE); + } } @Override public void running(IJobChangeEvent event) { - if (event.getJob() == running) + if (event.getJob() == running) { barrier.setStatus(TestBarrier.STATUS_RUNNING); - else if (event.getJob() == waiting) + } else if (event.getJob() == waiting) { barrier.setStatus(TestBarrier.STATUS_WAIT_FOR_DONE); + } } }; try { @@ -1339,11 +1460,12 @@ public class IJobManagerTest extends AbstractJobManagerTest { ISchedulingRule rule = new IdentityRule(); for (int i = 0; i < NUM_JOBS; i++) { //assign half the jobs to the first family, the other half to the second family - if (i % 2 == 0) + if (i % 2 == 0) { jobs[i] = new FamilyTestJob("TestFirstFamily", 1000000, 10, TestJobFamily.TYPE_ONE); - else + } else { /*if(i%2 == 1)*/ jobs[i] = new FamilyTestJob("TestSecondFamily", 1000000, 10, TestJobFamily.TYPE_TWO); + } jobs[i].setRule(rule); jobs[i].schedule(); @@ -1396,11 +1518,12 @@ public class IJobManagerTest extends AbstractJobManagerTest { ISchedulingRule rule = new IdentityRule(); for (int i = 0; i < NUM_JOBS; i++) { //assign half the jobs to the first family, the other half to the second family - if (i % 2 == 0) + if (i % 2 == 0) { jobs[i] = new FamilyTestJob("TestFirstFamily", 1000000, 10, TestJobFamily.TYPE_ONE); - else + } else { /*if(i%2 == 1)*/ jobs[i] = new FamilyTestJob("TestSecondFamily", 1000000, 10, TestJobFamily.TYPE_TWO); + } jobs[i].setRule(rule); jobs[i].schedule(); @@ -1501,17 +1624,20 @@ public class IJobManagerTest extends AbstractJobManagerTest { assertState("3.0", seedJob, Job.NONE); //all family jobs should still be sleeping - for (int i = 0; i < JOBS_PER_FAMILY; i++) + for (int i = 0; i < JOBS_PER_FAMILY; i++) { assertState("3.1." + i, family1[i], Job.SLEEPING); - for (int i = 0; i < JOBS_PER_FAMILY; i++) + } + for (int i = 0; i < JOBS_PER_FAMILY; i++) { assertState("3.2." + i, family2[i], Job.SLEEPING); + } //wake-up the second family of jobs manager.wakeUp(second); //jobs in the first family should still be in the sleep state - for (int i = 0; i < JOBS_PER_FAMILY; i++) + for (int i = 0; i < JOBS_PER_FAMILY; i++) { assertState("4.1." + i, family1[i], Job.SLEEPING); + } //ensure all jobs in second family are either running or waiting int runningCount = 0; for (int i = 0; i < JOBS_PER_FAMILY; i++) { @@ -1519,8 +1645,9 @@ public class IJobManagerTest extends AbstractJobManagerTest { if (state == Job.RUNNING) { runningCount++; } else { - if (state != Job.WAITING) + if (state != Job.WAITING) { assertTrue("4.2." + i + ": expected state: " + printState(Job.WAITING) + " actual state: " + printState(state), false); + } } } //ensure only one job is running (it is possible that none have started yet) @@ -1529,14 +1656,16 @@ public class IJobManagerTest extends AbstractJobManagerTest { //cycle through the jobs in the second family and cancel them for (int i = 0; i < JOBS_PER_FAMILY; i++) { //the running job may not respond immediately - if (!family2[i].cancel()) + if (!family2[i].cancel()) { waitForCancel(family2[i]); + } assertState("5." + i, family2[i], Job.NONE); } //all jobs in the first family should still be sleeping - for (int i = 0; i < JOBS_PER_FAMILY; i++) + for (int i = 0; i < JOBS_PER_FAMILY; i++) { assertState("6.1." + i, family1[i], Job.SLEEPING); + } //wake up the first family manager.wakeUp(first); @@ -1548,8 +1677,9 @@ public class IJobManagerTest extends AbstractJobManagerTest { if (state == Job.RUNNING) { runningCount++; } else { - if (state != Job.WAITING) + if (state != Job.WAITING) { assertTrue("7.1." + i + ": expected state: " + printState(Job.WAITING) + " actual state: " + printState(state), false); + } } } //ensure only one job is running (it is possible that none have started yet) @@ -1558,16 +1688,19 @@ public class IJobManagerTest extends AbstractJobManagerTest { //cycle through the jobs in the first family and cancel them for (int i = 0; i < JOBS_PER_FAMILY; i++) { //the running job may not respond immediately - if (!family1[i].cancel()) + if (!family1[i].cancel()) { waitForCancel(family1[i]); + } assertState("8." + i, family1[i], Job.NONE); } //all jobs should now be in the NONE state - for (int i = 0; i < JOBS_PER_FAMILY; i++) + for (int i = 0; i < JOBS_PER_FAMILY; i++) { assertState("9.1." + i, family1[i], Job.NONE); - for (int i = 0; i < JOBS_PER_FAMILY; i++) + } + for (int i = 0; i < JOBS_PER_FAMILY; i++) { assertState("9.2." + i, family2[i], Job.NONE); + } } public void testMutexRule() { @@ -1604,18 +1737,21 @@ public class IJobManagerTest extends AbstractJobManagerTest { IJobChangeListener listener = new JobChangeAdapter() { @Override public void done(IJobChangeEvent event) { - if (event.getJob() instanceof TestJob) + if (event.getJob() instanceof TestJob) { done.add(event.getJob()); + } } }; int[] sleepTimes = new int[] {50, 250, 500, 800, 1000, 1500}; Job[] jobs = new Job[sleepTimes.length]; manager.addJobChangeListener(listener); try { - for (int i = 0; i < sleepTimes.length; i++) + for (int i = 0; i < sleepTimes.length; i++) { jobs[i] = new TestJob("testOrder(" + i + ")", 1, 1); - for (int i = 0; i < sleepTimes.length; i++) + } + for (int i = 0; i < sleepTimes.length; i++) { jobs[i].schedule(sleepTimes[i]); + } waitForCompletion(); //make sure listener has had a chance to process the finished job while (done.size() != jobs.length) { @@ -1624,8 +1760,9 @@ public class IJobManagerTest extends AbstractJobManagerTest { } Job[] doneOrder = done.toArray(new Job[done.size()]); assertEquals("1.0", jobs.length, doneOrder.length); - for (int i = 0; i < doneOrder.length; i++) + for (int i = 0; i < doneOrder.length; i++) { assertEquals("1.1." + i, jobs[i], doneOrder[i]); + } } finally { manager.removeJobChangeListener(listener); } @@ -1637,19 +1774,22 @@ public class IJobManagerTest extends AbstractJobManagerTest { IJobChangeListener listener = new JobChangeAdapter() { @Override public void done(IJobChangeEvent event) { - if (event.getJob() instanceof TestJob) + if (event.getJob() instanceof TestJob) { //add at start of list to get reverse order done.add(0, event.getJob()); + } } }; int[] sleepTimes = new int[] {4000, 3000, 2000, 1000, 500}; Job[] jobs = new Job[sleepTimes.length]; manager.addJobChangeListener(listener); try { - for (int i = 0; i < sleepTimes.length; i++) + for (int i = 0; i < sleepTimes.length; i++) { jobs[i] = new TestJob("testReverseOrder(" + i + ")", 0, 1); - for (int i = 0; i < sleepTimes.length; i++) + } + for (int i = 0; i < sleepTimes.length; i++) { jobs[i].schedule(sleepTimes[i]); + } waitForCompletion(); //make sure listener has had a chance to process the finished job while (done.size() != jobs.length) { @@ -1658,8 +1798,9 @@ public class IJobManagerTest extends AbstractJobManagerTest { } Job[] doneOrder = done.toArray(new Job[done.size()]); assertEquals("1.0", jobs.length, doneOrder.length); - for (int i = 0; i < doneOrder.length; i++) + for (int i = 0; i < doneOrder.length; i++) { assertEquals("1.1." + i, jobs[i], doneOrder[i]); + } } finally { manager.removeJobChangeListener(listener); } @@ -1678,10 +1819,11 @@ public class IJobManagerTest extends AbstractJobManagerTest { try { synchronized (running) { //indicate job is running, and assert the job is not already running - if (running[0]) + if (running[0]) { failure[0] = true; - else + } else { running[0] = true; + } } //sleep for awhile to let duplicate job start running Thread.sleep(1000); @@ -1849,6 +1991,7 @@ public class IJobManagerTest extends AbstractJobManagerTest { * [Thread[Worker-3,5,main]]End rule: L/JUnit/junit/tests/framework/Failure.java * @deprecated tests deprecated API */ + @Deprecated public void testSuspendMismatchedBegins() { PathRule rule1 = new PathRule("/TestSuspendMismatchedBegins"); PathRule rule2 = new PathRule("/TestSuspendMismatchedBegins/Child"); @@ -1877,6 +2020,7 @@ public class IJobManagerTest extends AbstractJobManagerTest { * Tests IJobManager suspend and resume API * @deprecated tests deprecated API */ + @Deprecated public void testSuspendMultiThreadAccess() { PathRule rule1 = new PathRule("/TestSuspend"); PathRule rule2 = new PathRule("/TestSuspend/Child"); @@ -2033,8 +2177,9 @@ public class IJobManagerTest extends AbstractJobManagerTest { //let the destination finish barrier.setStatus(TestBarrier.STATUS_WAIT_FOR_DONE); waitForCompletion(destination); - if (!destination.getResult().isOK()) + if (!destination.getResult().isOK()) { fail("1.2", destination.getResult().getException()); + } } /** @@ -2085,8 +2230,9 @@ public class IJobManagerTest extends AbstractJobManagerTest { } catch (InterruptedException e) { fail("1.99", e); } - if (ender.error != null) + if (ender.error != null) { fail("1.0", ender.error); + } } /** @@ -2133,8 +2279,9 @@ public class IJobManagerTest extends AbstractJobManagerTest { manager.endRule(rule); //ensure the job didn't fail, and finally end the rule to unwind the initial beginRule - if (failure[0] != null) + if (failure[0] != null) { fail("1.0", failure[0]); + } try { manager.endRule(rule); } catch (Exception e) { @@ -2192,8 +2339,9 @@ public class IJobManagerTest extends AbstractJobManagerTest { waitForCompletion(job); //ensure the job didn't fail, and finally end the rule to assert we own it - if (failure[0] != null) + if (failure[0] != null) { fail("1.0", failure[0]); + } try { manager.endRule(rule); } catch (Exception e) { @@ -2249,8 +2397,9 @@ public class IJobManagerTest extends AbstractJobManagerTest { waitForCompletion(job); //ensure the job didn't fail, and finally end the rule to assert we own it - if (failure[0] != null) + if (failure[0] != null) { fail("1.0", failure[0]); + } try { manager.endRule(rule); } catch (Exception e) { |