IsCancellationRequested not being called

Question

I am trying to make a simple program to learn how to use cancellation tokens, but they seem to be giving me plenty of problems. By adjusting the CancelAfter value, I can absolutely stop a task dead, but IsCancellationRequested is not ever calling for me. Since CancelAfter seems to be executing properly, it's very confusing to me that IsCancellationRequested fails 100% of the time.

Here's the code: (note, it's a stripped down example)

The function that initializes the task:

void IPCheck() {
    var cToken = new CancellationTokenSource();
    cToken.CancelAfter(1000);

    string tempIP = "123.123.123.123";

    Task.Factory.StartNew(() => PingTask(tempIP, cToken.Token), cToken.Token);
}

The task that is successfully(?) canceled, but doesn't have IsCancellationRequested called.

void PingTask(string addressString, CancellationToken cancelToken) {

    Ping currentPing = new Ping();
    PingReply replyPing;
    replyPing = currentPing.Send(addressString);

    string returnString = "Return String Unmodified." + Environment.NewLine;


    if (replyPing.Status == IPStatus.Success) {
        returnString = "Ping to " + addressString + " successful." + Environment.NewLine);
    }

    else if (replyPing.Status == IPStatus.TimedOut) {
        returnString = "Ping to " + addressString + "timed out." + Environment.NewLine);
    }

    if (cancelToken.IsCancellationRequested) {
        returnString = "Cancellation Requested for " + addressString + Environment.NewLine);
    }

    TaskHelperReturnString(returnString);
    return;
}

Unless it succeeds or times out, while CancelAfter is set to something like 10000, returnString always writes "Return String Unmodified."

A successful ping will be over in an instant. With CancelAfter(1000) there isn't enough time for it to timeout and nothing is printed, including TaskHelperReturnString().

The IP I used always fails, so it's good for testing.

Answer 1

In all supported .NET versions (4.5.2+) you can use async/await and SendPingAsync to send a ping asynchronously without requiring Task.Run. Even without them, you don't need the cancellation source, as SendAsync(IPAddress,int) accepts a timeout parameter.

Your code could look like this :

async Task<string> IPCheck()
{
    Ping currentPing = new Ping();
    var replyPing= await currentPing.SendPingAsync(addressString,1000);        
    switch(replyPint.Status)
    {
        case IPStatus.Success:
            var okString = $"Ping to {addressString} successful.\n";
            SuccessActions(0, arrayIndex);
            return okString;
        case IPStatus.Timeout:
            return  $"Ping to {addressString} timed out.\n";
        default:
            // ????
    }
}

Even if you wanted a larger task timeout from the ping timeout (why?) you could use Task.Delay and Task.WhenAny instead of a cancellation source. A task timeout shorter than the ping timeout wouldn't make much sense, as you'd never get a ping timeout this way.

Using Task.Delay :

async Task<string> IPCheck()
{
    Ping currentPing = new Ping();

    var pingTask= currentPing.SendPingAsync(addressString,1000);        
    var delayTask=Task.Delay(2000);
    var finished =  await Task.WhenAny(delayTask,pingTask);
    if (finished == delayTask)
    {
        return "Long timeout!"
    }

    var replyPing=await pingTask;

    switch(replyPing.Status)
    {
        case IPStatus.Success:
            var okString = $"Ping to {addressString} successful.\n";
            SuccessActions(0, arrayIndex);
            return okString;
        case IPStatus.Timeout:
            return  $"Ping to {addressString} timed out.\n";
        default:
            // ????
    }
}

UPDATE

The original code can't work because the task is never awaited. The method where the cancellation source is defined exits immediatelly, forcing a garbage collection on the source. The source is not preserved even though the token is passed to the task. The token is. Since the source no longer exists, it can't notify any of the tokens it generated.

To make the original code work with a token source, the task itself has to be awaited or the cancellation source should have a larger scope than the method, ie it should be a field.

To make a proper example, assume we wanted 5 pings before responding Eg :

Update 2018-11-06 Added proper disposal of CancellationTokenSource to avoid flooding the timer queue. Explanation here

static async Task<string> IPCheck(string address)
{
    using (var cts = new CancellationTokenSource(2000))
    {
        var result = await SendPings(address, 5, cts.Token);
        return result;
    }
}


static async Task<string> SendPings(string address, int count, CancellationToken token)
{
    var responses = new List<PingReply>(5);

    Ping currentPing = new Ping();

    for (int i = 0; i < count; i++)
    {
        if (token.IsCancellationRequested)
        {
            return "Cancelled!";
        }

        var replyPing = await currentPing.SendPingAsync(address, 1000);
        switch (replyPing.Status)
        {
            case IPStatus.Success:
                responses.Add(replyPing);
                break;
            case IPStatus.TimedOut:
                responses.Add(replyPing);
                Console.WriteLine($"Ping to {address} timed out.\n");
                break;
            default:
                responses.Add(replyPing);
                break;
        }
    }
    var avgTime = responses.Average(resp => resp.RoundtripTime);
    return $"All pings returned. Average roundtrip {avgTime}";
}

In this case the IPCheck method won't block, but won't finish either until either all 5 pings finish in 2 seconds, or a timeout (ping or overall) occurs.

If the ping takes too long and the overall timeout is too short, IsCancellationRequested becomes true and Cancelled is returned.