Fix cancellation by polling example (#31643)

[JJS]

Summary

I moved the token.IsCancellationRequested-block out of the outer loop to ensure it is checked in all cases.

Fixes #31643

[BillWagner]

Thanks for updating this @JJS

I've reviewed these changes, and I'll now.

[JJS]

Is there something to do about the failed snippet? The error seems not to be related to the issue

[BillWagner]

Is there something to do about the failed snippet? The error seems not to be related to the issue

Yes, if you create a default .csproj and .vbproj in the two folders with these source files, that will fix the issue.

It's a historical artifact. The source code was imported into GitHub from an internal system that built the samples when edited. There weren't project files, and we've been adding them whenever we edit any of the existing samples.

[Youssef1313]

Is x in scope here?

[Youssef1313]

I don't see how cancellation is useful after the heavy work is already done?

[JJS]

true, x is not in scope, I'll fix that.

As the usefulness of the cancellation is concerned, the overall behavior of the code did not change.

The polling example's main part is the IsCancellationRequested check in the loop condition, which did not change.

I suppose you could remove the additional if block, which is only there to provide a place for clean up code in case of cancellation. But you might need a place to put the ThrowIfCancellationRequested call.

I am not sure what you mean by "heavy work is already done" as the outer loop can be left at any point when cancellation is requested.

[Youssef1313]

I could be misunderstanding something (and in fact I'm not sure what's the issue being resolved here), but I see token.IsCancellationRequested is done after the loops have already finished and completed the work.

This also doesn't make sense for me with the existing comment that says:

    // Assume that we know that the inner loop is very fast.
    // Therefore, checking once per row in the outer loop is sufficient.

What do you think @BillWagner?

[JJS]

I updated the scope, the comments and some other issues with mixed up columns/rows.

token.IsCancellationRequested is called in the loop condition (so polling each column), not only after the loops. So the loops may not have completed their work. As I understand the original example, the call to token.IsCancellationRequested after the loops is only for clean up or calling token.ThrowIfCancellationRequested.

As the linked issue correctly states, there is an unexpected behavior if the cancellation is requested after the if condition is evaluated and before the loop condition is evaluated. In this case, the clean up code is not called.

As I said, there is a point to removing the if block completely but there must be some place to put the token.ThrowIfCancellationRequested in.

[Youssef1313]

token.IsCancellationRequested is called in the loop condition

Ah I missed that.

[IEvangelist]

A few nits