Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAnton Leherbauer2015-04-23 11:33:26 +0000
committerAnton Leherbauer2015-04-23 12:35:06 +0000
commitcfa8c3b4d3b6f0815b4e8dc73d16dfbbafc10e28 (patch)
tree7182aadc5f6b569a797e9ed4cf8c7fe34a626cfc /target_explorer
parent05cfd00814ad6d0854ffdb6127b190217747695c (diff)
downloadorg.eclipse.tcf-cfa8c3b4d3b6f0815b4e8dc73d16dfbbafc10e28.tar.gz
org.eclipse.tcf-cfa8c3b4d3b6f0815b4e8dc73d16dfbbafc10e28.tar.xz
org.eclipse.tcf-cfa8c3b4d3b6f0815b4e8dc73d16dfbbafc10e28.zip
Target Explorer: Fix race conditions when canceling stepper jobs
- Avoid manipulating JobMonitor after job has finished - Spurious ConcurrentModificationException in CancelJobsStep Change-Id: Ia7baea03fc9f9ce093d9f26ba3e176e1405246ff Signed-off-by: Anton Leherbauer <anton.leherbauer@windriver.com>
Diffstat (limited to 'target_explorer')
-rw-r--r--target_explorer/plugins/org.eclipse.tcf.te.runtime.stepper/src/org/eclipse/tcf/te/runtime/stepper/internal/PropertyTester.java23
-rw-r--r--target_explorer/plugins/org.eclipse.tcf.te.runtime.stepper/src/org/eclipse/tcf/te/runtime/stepper/job/StepperJob.java96
-rw-r--r--target_explorer/plugins/org.eclipse.tcf.te.runtime.stepper/src/org/eclipse/tcf/te/runtime/stepper/steps/CancelJobsStep.java75
3 files changed, 110 insertions, 84 deletions
diff --git a/target_explorer/plugins/org.eclipse.tcf.te.runtime.stepper/src/org/eclipse/tcf/te/runtime/stepper/internal/PropertyTester.java b/target_explorer/plugins/org.eclipse.tcf.te.runtime.stepper/src/org/eclipse/tcf/te/runtime/stepper/internal/PropertyTester.java
index 540314f45..6c1de434b 100644
--- a/target_explorer/plugins/org.eclipse.tcf.te.runtime.stepper/src/org/eclipse/tcf/te/runtime/stepper/internal/PropertyTester.java
+++ b/target_explorer/plugins/org.eclipse.tcf.te.runtime.stepper/src/org/eclipse/tcf/te/runtime/stepper/internal/PropertyTester.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2011, 2014 Wind River Systems, Inc. and others. All rights reserved.
+ * Copyright (c) 2011, 2015 Wind River Systems, Inc. and others. All rights reserved.
* This program and the accompanying materials are made available under the terms
* of the Eclipse Public License v1.0 which accompanies this distribution, and is
* available at http://www.eclipse.org/legal/epl-v10.html
@@ -10,7 +10,6 @@
package org.eclipse.tcf.te.runtime.stepper.internal;
import java.util.List;
-import java.util.Map;
import org.eclipse.core.runtime.jobs.Job;
import org.eclipse.tcf.te.runtime.services.ServiceManager;
@@ -35,12 +34,10 @@ public class PropertyTester extends org.eclipse.core.expressions.PropertyTester
if ("isRunning".equals(property)) { //$NON-NLS-1$
if (operation != null) {
- Map<String,List<Job>> jobs = StepperJob.getJobs(receiver);
- if (jobs != null && jobs.containsKey(operation)) {
- for (Job job : jobs.get(operation)) {
- if (job instanceof StepperJob && !((StepperJob)job).isCanceled() && !((StepperJob)job).isFinished()) {
- return true;
- }
+ List<Job> jobs = StepperJob.getJobsForOperation(receiver, operation);
+ for (Job job : jobs) {
+ if (job instanceof StepperJob && !((StepperJob)job).isCanceled() && !((StepperJob)job).isFinished()) {
+ return true;
}
}
}
@@ -48,12 +45,10 @@ public class PropertyTester extends org.eclipse.core.expressions.PropertyTester
if ("isRunningOrCanceled".equals(property)) { //$NON-NLS-1$
if (operation != null) {
- Map<String,List<Job>> jobs = StepperJob.getJobs(receiver);
- if (jobs != null && jobs.containsKey(operation)) {
- for (Job job : jobs.get(operation)) {
- if (job instanceof StepperJob && !((StepperJob)job).isFinished()) {
- return true;
- }
+ List<Job> jobs = StepperJob.getJobsForOperation(receiver, operation);
+ for (Job job : jobs) {
+ if (job instanceof StepperJob && !((StepperJob)job).isFinished()) {
+ return true;
}
}
}
diff --git a/target_explorer/plugins/org.eclipse.tcf.te.runtime.stepper/src/org/eclipse/tcf/te/runtime/stepper/job/StepperJob.java b/target_explorer/plugins/org.eclipse.tcf.te.runtime.stepper/src/org/eclipse/tcf/te/runtime/stepper/job/StepperJob.java
index d9dad274c..3452e05a6 100644
--- a/target_explorer/plugins/org.eclipse.tcf.te.runtime.stepper/src/org/eclipse/tcf/te/runtime/stepper/job/StepperJob.java
+++ b/target_explorer/plugins/org.eclipse.tcf.te.runtime.stepper/src/org/eclipse/tcf/te/runtime/stepper/job/StepperJob.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2013, 2014 Wind River Systems, Inc. and others. All rights reserved.
+ * Copyright (c) 2013, 2015 Wind River Systems, Inc. and others. All rights reserved.
* This program and the accompanying materials are made available under the terms
* of the Eclipse Public License v1.0 which accompanies this distribution, and is
* available at http://www.eclipse.org/legal/epl-v10.html
@@ -10,6 +10,7 @@
package org.eclipse.tcf.te.runtime.stepper.job;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -18,6 +19,7 @@ import org.eclipse.core.runtime.Assert;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.ProgressMonitorWrapper;
import org.eclipse.core.runtime.Status;
import org.eclipse.core.runtime.jobs.IJobChangeEvent;
import org.eclipse.core.runtime.jobs.IJobChangeListener;
@@ -56,68 +58,85 @@ public final class StepperJob extends Job {
private boolean isCanceled = false;
private boolean statusHandled = false;
- private class NotCancelableProgressMonitor implements IProgressMonitor {
+ /**
+ * A progress monitor wrapper which blocks any access to a JobMonitor
+ * after the job has finished. This prevents creation of stale progress
+ * items in the ProgressManager.
+ */
+ private static class StepperProgressMonitor extends ProgressMonitorWrapper {
- private final IProgressMonitor monitor;
+ private final boolean cancelable;
+ private volatile boolean jobDone;
+ private volatile boolean canceled;
+
+ public StepperProgressMonitor(IProgressMonitor monitor, boolean isCancelable) {
+ super(monitor);
+ this.cancelable = isCancelable;
+ }
- public NotCancelableProgressMonitor(IProgressMonitor monitor) {
- this.monitor = monitor;
+ void jobDone(boolean canceled) {
+ // Job has finished - block any access to the JobMonitor
+ jobDone = true;
+ this.canceled = canceled;
}
@Override
public void beginTask(String name, int totalWork) {
- monitor.beginTask(name, totalWork);
+ if (!jobDone) super.beginTask(name, totalWork);
}
@Override
public void done() {
- monitor.done();
+ if (!jobDone) super.done();
}
@Override
public void internalWorked(double work) {
- monitor.internalWorked(work);
+ if (!jobDone) super.internalWorked(work);
}
@Override
public boolean isCanceled() {
- return false;
+ if (cancelable && !jobDone) return super.isCanceled();
+ return canceled;
}
@Override
public void setCanceled(boolean value) {
- monitor.setCanceled(false);
+ canceled = cancelable && value;
+ if (!jobDone) super.setCanceled(canceled);
}
@Override
public void setTaskName(String name) {
- monitor.setTaskName(name);
+ if (!jobDone) super.setTaskName(name);
}
@Override
public void subTask(String name) {
- monitor.subTask(name);
+ if (!jobDone) super.subTask(name);
}
@Override
public void worked(int work) {
- monitor.worked(work);
+ if (!jobDone) super.worked(work);
}
}
private class JobChangeListener extends JobChangeAdapter {
+ private final StepperProgressMonitor jobMonitor;
+
/**
* Constructor.
*/
- public JobChangeListener() {
+ public JobChangeListener(StepperProgressMonitor monitor) {
+ this.jobMonitor = monitor;
}
- /* (non-Javadoc)
- * @see org.eclipse.core.runtime.jobs.JobChangeAdapter#done(org.eclipse.core.runtime.jobs.IJobChangeEvent)
- */
@Override
public void done(IJobChangeEvent event) {
+ jobMonitor.jobDone(isCanceled());
handleStatus(event.getResult());
removeJobChangeListener(this);
@@ -132,7 +151,7 @@ public final class StepperJob extends Job {
* @param data The stepper data.
* @param stepGroupId The step group id to execute.
* @param operation The operation to execute.
- * @param isCancelable <code>true</code> if the job can be canceled.
+ * @param cancelable <code>true</code> if the job can be canceled.
* @param handleStatus <code>true</code> if the job should handle the status itself and return always <code>Status.OK_STATUS</code>.
*/
public StepperJob(String name, IStepContext stepContext, IPropertiesContainer data, String stepGroupId, String operation, boolean isCancelable, boolean handleStatus) {
@@ -182,20 +201,34 @@ public final class StepperJob extends Job {
return jobs;
}
+ public static List<Job> getJobsForOperation(Object context, String operation) {
+ Map<String, List<Job>> jobs = getJobs(context);
+ synchronized (jobs) {
+ List<Job> jobsForOperation = jobs.get(operation);
+ if (jobsForOperation == null)
+ return Collections.emptyList();
+ return new ArrayList<Job>(jobsForOperation);
+ }
+ }
+
public static void addJob(Map<String,List<Job>> jobs, Job job, String operation) {
- List<Job> jobsForOperation = jobs.get(operation);
- if (jobsForOperation == null) {
- jobsForOperation = new ArrayList<Job>();
- jobs.put(operation, jobsForOperation);
- }
- jobsForOperation.add(job);
+ synchronized (jobs) {
+ List<Job> jobsForOperation = jobs.get(operation);
+ if (jobsForOperation == null) {
+ jobsForOperation = new ArrayList<Job>();
+ jobs.put(operation, jobsForOperation);
+ }
+ jobsForOperation.add(job);
+ }
}
public static void removeJob(Map<String,List<Job>> jobs, Job job, String operation) {
- List<Job> jobsForOperation = jobs.get(operation);
- if (jobsForOperation != null) {
- jobsForOperation.remove(job);
- }
+ synchronized (jobs) {
+ List<Job> jobsForOperation = jobs.get(operation);
+ if (jobsForOperation != null) {
+ jobsForOperation.remove(job);
+ }
+ }
}
public static void setJobs(Map<String,List<Job>> jobs, Object context) {
@@ -234,11 +267,10 @@ public final class StepperJob extends Job {
@Override
public final IStatus run(IProgressMonitor monitor) {
- if (!isCancelable) {
- monitor = new NotCancelableProgressMonitor(monitor);
- }
+ StepperProgressMonitor stepperMonitor = new StepperProgressMonitor(monitor, isCancelable);
+ monitor = stepperMonitor;
- IJobChangeListener listener = new JobChangeListener();
+ IJobChangeListener listener = new JobChangeListener(stepperMonitor);
addJobChangeListener(listener);
// The stepper instance to be used
diff --git a/target_explorer/plugins/org.eclipse.tcf.te.runtime.stepper/src/org/eclipse/tcf/te/runtime/stepper/steps/CancelJobsStep.java b/target_explorer/plugins/org.eclipse.tcf.te.runtime.stepper/src/org/eclipse/tcf/te/runtime/stepper/steps/CancelJobsStep.java
index a199b4634..7dc1087ed 100644
--- a/target_explorer/plugins/org.eclipse.tcf.te.runtime.stepper/src/org/eclipse/tcf/te/runtime/stepper/steps/CancelJobsStep.java
+++ b/target_explorer/plugins/org.eclipse.tcf.te.runtime.stepper/src/org/eclipse/tcf/te/runtime/stepper/steps/CancelJobsStep.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2013, 2014 Wind River Systems, Inc. and others. All rights reserved.
+ * Copyright (c) 2013, 2015 Wind River Systems, Inc. and others. All rights reserved.
* This program and the accompanying materials are made available under the terms
* of the Eclipse Public License v1.0 which accompanies this distribution, and is
* available at http://www.eclipse.org/legal/epl-v10.html
@@ -10,6 +10,8 @@
package org.eclipse.tcf.te.runtime.stepper.steps;
+import java.util.ArrayList;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;
@@ -57,46 +59,43 @@ public class CancelJobsStep extends AbstractStep {
Job thisJob = (StepperJob)StepperAttributeUtil.getProperty(IStepAttributes.ATTR_STEPPER_JOB, fullQualifiedId, data);
final AsyncCallbackCollector collector = new AsyncCallbackCollector();
- Map<String,List<Job>> jobs = StepperJob.getJobs(context.getContextObject());
- final AtomicInteger numJobsToCancel = new AtomicInteger(0);
+ Map<String,List<Job>> jobs = new HashMap<String, List<Job>>(StepperJob.getJobs(context.getContextObject()));
final AtomicInteger canceledJobs = new AtomicInteger(0);
- for (String op : jobs.keySet()) {
- for (Job job : jobs.get(op)) {
- if (job != thisJob &&
- (!(job instanceof StepperJob) || ((StepperJob)job).isCancelable())) {
- numJobsToCancel.set(numJobsToCancel.get()+1);
- }
+ final List<Job> jobsToCancel = new ArrayList<Job>();
+ synchronized (jobs) {
+ for (String op : jobs.keySet()) {
+ for (Job job : jobs.get(op)) {
+ if (job != thisJob &&
+ (!(job instanceof StepperJob) || ((StepperJob)job).isCancelable())) {
+ jobsToCancel.add(job);
+ }
+ }
}
}
- for (String op : jobs.keySet()) {
- for (Job job : jobs.get(op)) {
- if (job != thisJob) {
- if (job instanceof StepperJob && ((StepperJob)job).isCancelable()) {
- Callback jobCb = new Callback(((StepperJob)job).getJobCallback()) {
- @Override
- protected void internalDone(Object caller, IStatus status) {
- canceledJobs.set(canceledJobs.get()+1);
- ProgressHelper.worked(monitor, getTotalWork(context, data) / numJobsToCancel.get());
- ProgressHelper.setSubTaskName(monitor, canceledJobs.get() + " of " + numJobsToCancel.get() + " Jobs canceled."); //$NON-NLS-1$ //$NON-NLS-2$
- collector.removeCallback(this);
- }
- };
- if (job.getState() == Job.RUNNING) {
- collector.addCallback(jobCb);
- ((StepperJob)job).setJobCallback(jobCb);
- }
- else {
- canceledJobs.set(canceledJobs.get()+1);
- }
- }
- else {
- canceledJobs.set(canceledJobs.get()+1);
- }
- job.cancel();
- }
- }
-
- }
+ for (Job job : jobsToCancel) {
+ if (job instanceof StepperJob && ((StepperJob)job).isCancelable()) {
+ Callback jobCb = new Callback(((StepperJob)job).getJobCallback()) {
+ @Override
+ protected void internalDone(Object caller, IStatus status) {
+ canceledJobs.incrementAndGet();
+ ProgressHelper.worked(monitor, getTotalWork(context, data) / jobsToCancel.size());
+ ProgressHelper.setSubTaskName(monitor, canceledJobs.get() + " of " + jobsToCancel.size() + " Jobs canceled."); //$NON-NLS-1$ //$NON-NLS-2$
+ collector.removeCallback(this);
+ }
+ };
+ if (job.getState() == Job.RUNNING) {
+ collector.addCallback(jobCb);
+ ((StepperJob)job).setJobCallback(jobCb);
+ }
+ else {
+ canceledJobs.incrementAndGet();
+ }
+ }
+ else {
+ canceledJobs.incrementAndGet();
+ }
+ job.cancel();
+ }
collector.initDone();

Back to the top