Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

SemaphoreSlim Cancellation Token

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
like image 974
cubesnyc Avatar asked Jun 07 '14 00:06

cubesnyc


1 Answers

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

  1. If I can, i prefer checking for cancellation and ending the operation, and not throwing an exception.
  2. 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.
  3. To parallelize the site updates, fire all updates and await only once with Task.WhenAll
  4. 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.
  5. 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.

like image 79
i3arnon Avatar answered Oct 01 '22 10:10

i3arnon