Why isn't it possible to send a cancel request to a JobGroup by calling IProgressMonitor::setCanceled from one of its Jobs?

[fedejeanne]

Hello all,
I was wondering why isn't it possible to cancel a whole send a cancel request to a JobGroup by calling monitor.setCanceled(true) inside the run method of one of the Jobs in the group?

If I do something like this:

IProgressMonitor progressGroup = Job.getJobManager().createProgressGroup();
progressGroup.beginTask("", 2);
job1.setProgressGroup(progressGroup, 1);
job2.setProgressGroup(progressGroup, 1);

JobGroup jobGroup = new JobGroup("The group", 0, 0);
job1.setJobGroup(jobGroup);
job2.setJobGroup(jobGroup);

job1.schedule(); // this one cancels its monitor
job1.join();

job2.schedule(); // here, monitor.isCanceled() == false

Since both jobs have completely unrelated monitors, they can not "pass" the state of their monitor to one other. This means that to cancel all jobs in the group one can only call jobGroup.cancel() and implement the hook method Job::canceling in each job.

Wouldn't it be better to have all Monitors in the jobs related to each other so they can know if some other job in the group canceled its execution without having to implement an extra method (and use an extra field)?

[HeikoKlare]

I was wondering why isn't it possible to cancel a whole JobGroup by calling monitor.setCanceled(true) inside the run method of one of the Jobs in the group?

Note that monitor.setCanceled() does not cancel anything but only performs a cancel request. Since the Job is the one reporting to the monitor (and retrieving cancel requests from it), it does not make sense to report a cancel request to the monitor from within the Job, as the Job is the one to decide about whether to process the request or not anyway.

In addition, a Job can be put into any JobGroup. Whether or not cancelling one Job should also apply to other Jobs in its JobGroup depends on the semantics of the containing JobGroup. So without considering the JobGroup's semantics, it is impossible to decide whether a cancel request to the monitor given to a Job shall be transferred to other Jobs in that JobGroup. You need some entity that decides about the semantics of the cancel event w.r.t. to the JobGroup in which the Job is contained.

Maybe (at least from a functional perspective) setProgressGroup does something related to your expectation?

eclipse.platform/runtime/bundles/org.eclipse.core.jobs/src/org/eclipse/core/runtime/jobs/Job.java

Lines 733 to 735 in eb98dc4

	public final void setProgressGroup(IProgressMonitor group, int ticks) {
	super.setProgressGroup(group, ticks);
	}

[fedejeanne]

Note that monitor.setCanceled() does not cancel anything but only performs a cancel request

Thank you for pointing that out, I updated the text and the title.

The point is that there is no way to communicate the cancel requests that happened in one Monitor to all other Monitors, not even by calling progressGroup.setCanceled(true) right after its creation in the previous snippet.

On the other hand, if I try something similar with SubMonitors, they do what I would expect the group monitor to do (that is: canceling the parent or a sibling monitor reflects on all monitors). You can see it in these 2 tests (they both pass):

public void testCancelParentMonitor_childMonitorIsCanceled() {
	NullProgressMonitor parent = new NullProgressMonitor();
	SubMonitor child1 = SubMonitor.convert(parent);

	parent.setCanceled(true);

	assertTrue(child1.isCanceled());
}

public void testCancelChildMonitor_allMonitorsAreCanceled() {
	NullProgressMonitor parent = new NullProgressMonitor();
	SubMonitor child1 = SubMonitor.convert(parent);
	SubMonitor child2 = SubMonitor.convert(parent);

	child1.setCanceled(true);

	assertTrue(parent.isCanceled());
	assertTrue(child2.isCanceled());
}

[HeikoKlare]

Okay, I agree that one would expect monitors of Jobs within one ProgressGroups to behave like a hierarchy of SubMonitors. The specifications of the according interfaces and classes (in particular the ProgressProvider) also imply that.

From my point of view, the current implementation of the JobManager is not correct. When starting a job, it always creates and sets a new monitor:

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

Line 1788 in eb98dc4

internal.setProgressMonitor(createMonitor(j));

This always overwrites a monitor that may have already been set when assigning the Job to a progress group. This also conflicts with the specification of the called method, which says that it may only be called when the Job is not part of a job group. So I would expect this line of code to be wrapped inside a check for the Job being part of a progress group (i.e., for no monitor being already set, which happens when assigning the job to a progress group).

I think this probably incorrect default behavior will usually not be perceived, because the Eclipse workbench uses a custom ProgressProvider implementation ( ProgressManager) that implements all the related methods on its own, see here. It ensures that for every Job only a single monitor is instantiated and properly handled in terms of having a progress group. This is also conflicting with the specification of the specialized ProgressProvider, which states that the method returns a new monitor instance, whereas the implementation actually always return the same one when called multiple times.

[fedejeanne]

Sorry for the late reply: I provided a PR to fix the first issue (#666) i.e. cancelation of the jobs when canceling the whole progress group.

Regarding the second issue (cancelation of "sibling jobs") IMO the desired behavior wasn't really specified in the JavaDocs and I think I know why: in the UI, jobs in a group are bundled together in the same ProgressInfoItem ...

... and the cancelation occurs via the method org.eclipse.ui.internal.progress.ProgressInfoItem.cancelOrRemove(), which calls org.eclipse.ui.internal.progress.GroupInfo.cancel().
https://github.com/eclipse-platform/eclipse.platform.ui/blob/73307cb811a2dbdc8023522e1fda6fcee6dba4d0/bundles/org.eclipse.ui.workbench/Eclipse%20UI/org/eclipse/ui/internal/progress/GroupInfo.java#L187-L193

This means that, when working via the UI, one never really cancels one of the jobs but the whole group (JobGroup) by pressing the red button on the right and everything works as desired. When working via the code though (i.e. inside one of the jobs in the group), it's a different story: one needs to work with the JobGroup and specify the desired cancelation policy. These tests illustrate what the default policy is and how can one override it:

• org.eclipse.core.tests.runtime.jobs.JobGroupTest.testShouldCancel_1(): illustrates the default policy i.e. canceling the group only after one of its jobs returns an ERROR state
• org.eclipse.core.tests.runtime.jobs.JobGroupTest.testShouldCancel_2(): shows the inner works of the method shouldCancel
• org.eclipse.core.tests.runtime.jobs.JobGroupTest.testShouldCancel_3(): also shows the inner works of the method shouldCancel but more in detail
• org.eclipse.core.tests.runtime.jobs.JobGroupTest.testShouldCancel_4(): overrides the default policy and cancels the group after certain jobs have been completed.

This means that there is no need to really change the current API in order to cancel "sibling" jobs, one can simply provide the desired cancelation policy in the JobGroup. I provided a PR with a test that shows exactly that (#675)