SemaphoreSlim Cancellation Token

Question

class Program
{
    static IEnumerable<site> list = Enumerable.Range(1, 10).Select(i => new site(i.ToString()));

    static void Main(string[] args)
    {
        startup();
        Console.ReadKey();
    }

    static public void startup()
    {
        router.cts = new CancellationTokenSource();

        foreach (var s in list)
        {
            update(s);
        }
    }

    async static public void update(site s)
    {
        try
        {
            while (true)
            {
                await s.Refresh();
                if (site.count % 4 == 0)
                {
                    Console.WriteLine("Reseting Queue");
                    router.cts.Cancel();
                }
            }
        }
        catch (OperationCanceledException)
        {
            Console.WriteLine("Canceled");
            startup();
        }
    }
}

class router
{
    public static SemaphoreSlim ss = new SemaphoreSlim(1);
    public static CancellationTokenSource cts { get; set; }


}

class site
{
    public static int count = 0;
    public string sitename {get; set;}

    public site(string s)
    {
        sitename = s;
    }

    async public Task Refresh()
    {
        await router.ss.WaitAsync(router.cts.Token);
        //Console.WriteLine("{0}:: Start Refreshing ", sitename);
        await Task.Delay(1000);

        Console.WriteLine("{0}:: Done Refreshing ", sitename);
        count++;
        router.ss.Release();
    }


}

I am trying to mimic a pattern that launches an infinite while loop that simulates a constant updating of a site. I am mimicking this with the modulus. In theory I would like this to cancel all of the tasks queued by the semaphore and restart the queue from the beginning but it doesn't seem to be doing that. Could somebody please comment on my logic and pattern?

the output right now looks like this::

1:: Done Refreshing
2:: Done Refreshing
3:: Done Refreshing
4:: Done Refreshing
Reseting Queue
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
5:: Done Refreshing
1:: Done Refreshing
2:: Done Refreshing
3:: Done Refreshing
Reseting Queue
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
4:: Done Refreshing
5:: Done Refreshing
6:: Done Refreshing
7:: Done Refreshing
Reseting Queue
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
Canceled
8:: Done Refreshing
9:: Done Refreshing
10:: Done Refreshing
1:: Done Refreshing
Reseting Queue
Canceled

Do you want the sites to be updated in parallel, or one by one? — i3arnon, Jun 07 '14 at 01:19
I want them to update in parallel but the semaphore is to simulate a code that needs to have a lock on it. — cubesnyc, Jun 07 '14 at 01:34
@YevgenyYurgenson, is this a follow-up to your [previous question](http://stackoverflow.com/q/24020740/1768303)? What's the point of having `SemaphoreSlim(1)`? Do you not want to have more than 1 operation at the same time? — noseratio, Jun 07 '14 at 02:24
Yes it is a follow up. The point of the semaphoreslim is to mimic an operation that needs to be locked. The code is just a small case study to see how best to code a project I am working on. — cubesnyc, Jun 07 '14 at 13:47

i3arnon · Accepted Answer · 2014-06-07T17:40:28.793

So, I have several comments, some are bugs and some just suggestions:

If I can, i prefer checking for cancellation and ending the operation, and not throwing an exception.
update is async void, something that should almost never happen outside an event handler. You can't observe errors and it can cause a myriad of bugs.
To parallelize the site updates, fire all updates and await only once with Task.WhenAll
You are calling startup when any operation is cancelled. That means that when you cancell 5 of the 10 site updates you're launching 50 new site updates. That is unnecessary.
Passing a CancellationToken to the SemaphoreSlim.WaitAsync only observes cancellation while waiting for the semaphore. any operation already running will not stop. Is that really your intent? A better approach would be to check the token while updating. This can be simulated by passing the token to the Task.Delay operation.

Here's how I would have done it:

class Program
{
    static IEnumerable<site> list = Enumerable.Range(1, 10).Select(i => new site(i.ToString()));

    static void Main(string[] args)
    {
        Startup().Wait();
        Console.ReadKey();
    }

    static async Task Startup()
    {
        while (true)
        {
            router.cts = new CancellationTokenSource();

            await Task.WhenAll(list.Select(s => Update(s)));
        }
    }

    static Task Update(site s)
    {
        if (site.count % 4 == 0)
        {
            Console.WriteLine("Reseting Queue");
            router.cts.Cancel();
        }
        else
        {
            return s.Refresh();
        }
    }
}

class router
{
    public static SemaphoreSlim ss = new SemaphoreSlim(1);
    public static CancellationTokenSource cts { get; set; }
}

class site
{
    public static int count = 0;
    public string sitename {get; set;}

    public site(string s)
    {
        sitename = s;
    }

    public async Task Refresh()
    {
        await router.ss.WaitAsync();
        try
        {
            if (router.cts.token.IsCancellationRequested)
            {
                return;
            }
            await Task.Delay(500);

            if (router.cts.token.IsCancellationRequested)
            {
                return;
            }
            await Task.Delay(500);

            Console.WriteLine("{0}:: Done Refreshing ", sitename);
            count++;
        }
        finally
        {
            router.ss.Release();
        }
    }
}

I have split the Task.Delay to make it more similar, in my opinion, to a real case where you have several different operations (download, parse, save for example) and you want to query the cancellation token between those steps.

SemaphoreSlim Cancellation Token

1 Answers1