SlicedProgressMonitor: fix canceled()

[jukzi]

as soon as is started using sliced() cancel test fails because cancel is not forwarded:

org.eclipse.ltk.core.refactoring.tests.participants.CancelingParticipantTests

[jukzi]

see https://ci.eclipse.org/platform/job/eclipse.platform.ui/job/PR-1397/1/testReport/junit/AllTests%20org.eclipse.ltk.core.refactoring.tests.participants.ParticipantTests%20org.eclipse.ltk.core.refactoring.tests.participants/CancelingParticipantTests/testCreatePreChange/

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertTrue(Assert.java:53)
	at org.eclipse.ltk.core.refactoring.tests.participants.CancelingParticipantTests.testCreatePreChange(CancelingParticipantTests.java:207)

[jukzi]

@laeubi

[laeubi]

if you want this you need to synchronize on the monitor instance, beside that one usually don't want to cancel a slice, what is the use-case for that? According to the javadoc a monitor passed to a method should actually never call cancel so I'm not sure what these cancel tests "test"?

[laeubi]

Just to give some context, slice() is to create an independent monitor in case of using each on different concurrent threads, so canceling should only cancel the current thread but not all threads.

You probably want to use SubMonitor.split(...) instead for the non-concurrent case (what cancels the parent as well).

[jukzi]

You probably want to use SubMonitor.split(...) instead for the non-concurrent case (what cancels the parent as well).

I don't want to use SubMonitor.split(...) because it requires to construct a Submonitor instance before which does not refactor as easy. If split() works on multiple threads it should also work on single threads.

But ok: i see org.eclipse.core.runtime.IProgressMonitor.slice(int) javadoc indeed specified cancel() is not passed. How useless - at least for my usecase :-(

[laeubi]

I don't want to use SubMonitor.split(...) because it requires to construct a Submonitor instance before which does not refactor as easy.

I must confess I don't understand you call SubMonitor.convert(IProgressMonitor) and then have it "constructed" so this is usually the very first call and then you can split it as you like.

If split() works on multiple threads it should also work on single threads.

Please read the docs, split() does not work on multiple threads, the returned monitor instance is suitable to be passed to other than the current thread, but you need to join later on, e.g on a parallel pipeline.

i see org.eclipse.core.runtime.IProgressMonitor.slice(int) javadoc indeed specified cancel() is not passed.

As explained, the cancel is to allow to cancel the one computation forked in a different thread, if you want to cancel all you need to call cancel on the monitor where you have started the split.

How useless - at least for my usecase :-(

For your usecase there is already SubMonitor.convert(IProgressMonitor) + split(...) what don't work for concurrent case as it split commits all previously splitted work ...

[jukzi]

I must confess I don't understand you call SubMonitor.convert(IProgressMonitor) and then have it "constructed" so this is usually the very first call and then you can split it as you like.

if you do it on new code - ok. but we have hundrets of calls where i found no pattern convert all at once :-(

[laeubi]

if you do it on new code - ok. but we have hundrets of calls where i found no pattern convert all at once :-(

I usually do the following:

1. rename (with refactoring) the IProgressMonitor parameter of the method to "submonitor"
2. now rename (without refactoring) the IProgressMonitor parameter of the method to "monitor"
3. create a new local variable as SubMonitor submonitor = SubMonitor.convert(monitor)

maybe this can be automated with rewrite-maven-plugin one day...