Simplify TaskExtensions.IsCompletedSuccessfully(this Task t)

[drieseng]

In TaskExtensions.IsCompletedSuccessfully(this Task t) we currently consider the task to be completed successfully if all of the following conditions are met:

• Status is RanToCompletion.
• IsFaulted is false.
• IsCanceled is false.

These last two conditions are not necessary as Status is only RanToCompletion if the task completed successfully.

I added unit tests for TaskExtensions.IsCompletedSuccessfully(this Task t), but they may require a little tuning to get them to produce stable results.

[lahma]

Should we just keep the old expression and short-circuit for the fast one on .NET Core?

[drieseng]

Are you afraid it's not correct?
I was already convinced from the docs (RanToCompletion = "The task completed execution successfully."), but added unit tests just to be 100% sure.

[lahma]

Mmm.. why would there be new IsCompletedSuccessfully if it's the same thing? Looking at the sources it checks a bit different flags.

[drieseng]

These have an underlying flags enum (called m_stateFlags). In that enum you can have the RanToCompletion flag set in a combination with Canceled or Faulted.

Which is why in Status, they have the following logic:

if ((sf & (int)TaskStateFlags.Faulted) != 0) rval = TaskStatus.Faulted;
else if ((sf & (int)TaskStateFlags.Canceled) != 0) rval = TaskStatus.Canceled;
else if ((sf & (int)TaskStateFlags.RanToCompletion) != 0) rval = TaskStatus.RanToCompletion;
...

You'll only get RanToCompletion as Status if neither the Faulted nor Canceled flags where set on the underlying (flags) enum.

The IsCompletedSuccessfully property was introduced for convenience, and it should be a little faster since they only perform one check to see if RanToCompletion is the only flag - of the Canceled, Faulted and RanToCompletion flags - that is set on the underlying flags enum:

public bool IsCompletedSuccessfully => (m_stateFlags & (int)TaskStateFlags.CompletedMask) == (int)TaskStateFlags.RanToCompletion;