Don't replace the monitor set by a group when starting a new job

- Only set the progress monitor of the Job if it hasn't one already.
This makes sure that the progress monitor set via Job::setProgressGroup
is not replaced with a new one and ergo when the group is canceled, all
the monitors in that group are canceled too.

- Add regression test:
JobTest.testSetProgressGroup_progressGroupIsCanceled_monitorInJobIsCanceled()

- Simplify private method JobManager::createMonitor(Job). Remove the
annotation "@GuardedBy" in it.

Fixes #664

runtime/bundles/org.eclipse.core.jobs/META-INF/MANIFEST.MF

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.core.jobs; singleton:=true
-Bundle-Version: 3.15.0.qualifier
+Bundle-Version: 3.15.100.qualifier
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
 Export-Package: org.eclipse.core.internal.jobs;x-internal:=true,

runtime/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/JobManager.java

@@ -554,15 +554,12 @@ private <T, J extends InternalJob> T withWriteLock(J job, Function<J, T> functio
 
 	/**
 	 * Returns a new progress monitor for this job.  Never returns null.
-	 * @GuardedBy("lock")
 	 */
 	private IProgressMonitor createMonitor(Job job) {
-		IProgressMonitor monitor = null;
-		if (progressProvider != null)
-			monitor = progressProvider.createMonitor(job);
-		if (monitor == null)
-			monitor = new NullProgressMonitor();
-		return monitor;
+		if (progressProvider != null) {
+			return progressProvider.createMonitor(job);
+		}
+		return new NullProgressMonitor();
 	}
 
 	@Override
@@ -1785,7 +1782,10 @@ protected Job startJob(Worker worker) {
 				synchronized (internal.jobStateLock) {
 					if (internal.internalGetState() == InternalJob.ABOUT_TO_RUN) {
 						if (shouldReallyRun && !internal.isAboutToRunCanceled()) {
-							internal.setProgressMonitor(createMonitor(j));
+							// only set the monitor if it wasn't already set.
+							if (internal.getProgressMonitor() == null) {
+								internal.setProgressMonitor(createMonitor(j));
+							}
 							//change from ABOUT_TO_RUN to RUNNING
 							internal.setThread(worker);
 							internal.internalSetState(Job.RUNNING);

runtime/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/JobTest.java

@@ -23,11 +23,29 @@
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
-import java.util.concurrent.atomic.*;
-import org.eclipse.core.internal.jobs.*;
-import org.eclipse.core.runtime.*;
-import org.eclipse.core.runtime.jobs.*;
-import org.eclipse.core.tests.harness.*;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicIntegerArray;
+import java.util.concurrent.atomic.AtomicLong;
+import org.eclipse.core.internal.jobs.InternalJob;
+import org.eclipse.core.internal.jobs.JobListeners;
+import org.eclipse.core.internal.jobs.JobManager;
+import org.eclipse.core.internal.jobs.Worker;
+import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.OperationCanceledException;
+import org.eclipse.core.runtime.QualifiedName;
+import org.eclipse.core.runtime.Status;
+import org.eclipse.core.runtime.jobs.IJobChangeEvent;
+import org.eclipse.core.runtime.jobs.IJobChangeListener;
+import org.eclipse.core.runtime.jobs.IJobManager;
+import org.eclipse.core.runtime.jobs.ISchedulingRule;
+import org.eclipse.core.runtime.jobs.Job;
+import org.eclipse.core.runtime.jobs.JobChangeAdapter;
+import org.eclipse.core.runtime.jobs.LockListener;
+import org.eclipse.core.tests.harness.FussyProgressMonitor;
+import org.eclipse.core.tests.harness.TestBarrier2;
+import org.eclipse.core.tests.harness.TestJob;
 import org.junit.Assert;
 
 /**
@@ -1602,6 +1620,38 @@ protected IStatus run(IProgressMonitor monitor) {
 		group.done();
 	}
 
+	public void testSetProgressGroup_progressGroupIsCanceled_monitorInJobIsCanceled() throws InterruptedException {
+		Job checkMonitorIsCanceled = createJobThatChecksItsOwnMonitor();
+
+		final IProgressMonitor progressGroup = Job.getJobManager().createProgressGroup();
+		progressGroup.beginTask("", 2);
+
+		checkMonitorIsCanceled.setProgressGroup(progressGroup, 1);
+
+		// let the job do the cancellation
+		progressGroup.setCanceled(true);
+
+		checkMonitorIsCanceled.schedule();
+		checkMonitorIsCanceled.join();
+
+		assertEquals("The job should have been canceled", IStatus.CANCEL, checkMonitorIsCanceled.getResult().getSeverity());
+	}
+
+	private Job createJobThatChecksItsOwnMonitor() {
+		return new Job("Check if own monitor is canceled") {
+
+			@Override
+			protected IStatus run(IProgressMonitor monitor) {
+
+				if (monitor.isCanceled()) {
+					return Status.CANCEL_STATUS;
+				}
+
+				return Status.OK_STATUS;
+			}
+		};
+	}
+
 	/*
 	 * see bug #43459
 	 */