Emixam23
Emixam23

Reputation: 3964

Cancellable SemaphoreSlim - Would you suggest any amelioration for my encapsulation?

I need for X reason to write an encapsulation of semaphore that would allow me to cancel all the waiting processus on my SemaphoreSlim. (SemaphoreSlim Cancellation Encapsulation)

There is my class:

public class CancellableSemaphoreSlim
{
    readonly Queue<CancellationTokenSource> tokens = new Queue<CancellationTokenSource>();
    readonly SemaphoreSlim ss;

    /// <summary>
    /// Initializes a new instance of the <see cref="T:Eyes.Mobile.Core.Helpers.CancellableSemaphoreSlim"/> class.
    /// </summary>
    /// <param name="initialCount">Initial count.</param>
    public CancellableSemaphoreSlim(int initialCount) { ss = new SemaphoreSlim(initialCount); }

    /// <summary>Asynchronously waits to enter the <see cref="T:System.Threading.SemaphoreSlim" />, while observing a <see cref="T:System.Threading.CancellationToken" />. </summary>
    /// <returns>A task that will complete when the semaphore has been entered. </returns>
    /// <exception cref="T:System.ObjectDisposedException">The current instance has already been disposed.</exception>
    /// <exception cref="T:System.OperationCanceledException" />
    public Task WaitAsync()
    {
        CancellationTokenSource cancellationTokenSource = new CancellationTokenSource();
        tokens.Enqueue(cancellationTokenSource);
        return ss.WaitAsync(cancellationTokenSource.Token);
    }

    /// <summary>Asynchronously waits to enter the <see cref="T:System.Threading.SemaphoreSlim" />, while observing a <see cref="T:System.Threading.CancellationTokenSource" />. </summary>
    /// <returns>A task that will complete when the semaphore has been entered. </returns>
    /// <param name="cancellationTokenSource">The <see cref="T:System.Threading.CancellationToken" /> token to observe.</param>
    /// <exception cref="T:System.ObjectDisposedException">The current instance has already been disposed.</exception>
    /// <exception cref="T:System.OperationCanceledException">
    ///     <paramref name="cancellationTokenSource" /> was canceled. 
    /// </exception>
    public Task WaitAsync(CancellationToken cancellationToken)
    {
        CancellationTokenSource cancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
        tokens.Enqueue(cancellationTokenSource);
        return ss.WaitAsync(cancellationTokenSource.Token);
    }

    /// <summary>
    /// Release this instance.
    /// </summary>
    /// <returns>The released semaphore return.</returns>
    public int Release() => ss.Release();

    /// <summary>
    /// Cancel all processus currently in WaitAsync() state.
    /// </summary>
    public void CancelAll()
    {
        while (tokens.Count > 0)
        {
            CancellationTokenSource token = tokens.Dequeue();
            if (!token.IsCancellationRequested)
                token.Cancel();
        }
    }
}

You can use it like a basic SemaphoreSlim, I wrote a simple sample:

class Program
{
    static void Main(string[] args)
    {
        AsyncContext.Run(() => MainAsync(args));
    }

    static async void MainAsync(string[] args)
    {
        for (int i = 0; i < 5; i++)
        {
            try
            {
                CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(10000);
                await Task.WhenAll(
                    MakeAnAction(i, cancellationTokenSource),
                    MakeAnAction(i, cancellationTokenSource),
                    MakeAnAction(i, cancellationTokenSource),
                    MakeAnAction(i, cancellationTokenSource),
                    MakeAnAction(i, cancellationTokenSource)
                    );
            }
            catch (OperationCanceledException) { }
        }
        await Task.Delay(5000);
        cancellableSemaphoreSlim.CancelAll();
        await Task.Delay(5000);
    }

    readonly static CancellableSemaphoreSlim cancellableSemaphoreSlim = new CancellableSemaphoreSlim(1);
    readonly static Random rnd = new Random();

    internal static async Task MakeAnAction(int id, CancellationTokenSource cancellationTokenSource)
    {
        try
        {
            await cancellableSemaphoreSlim.WaitAsync(cancellationTokenSource.Token);
            int actionTime = rnd.Next(2, 10) * 1000;
            Output($"{id} : Start ({actionTime})");
            await Task.Delay(actionTime, cancellationTokenSource.Token);
            Output($"{id} : OK ({actionTime})");
        }
        catch (OperationCanceledException)
        {
            Output($"{id} : Cancelled");
        }
        finally
        {
            cancellableSemaphoreSlim.Release();
        }
    }

    private static void Output(string str)
    {
        Debug.WriteLine(str);
        Console.WriteLine(str);
    }
}

However, I was wondering if using a Queue<CancellationTokenSource> could create any asynchronous problem? Because, if we have a method (makeAnAction like) which can be call by different Threads/Tasks, if CancelAll() is called before the new Task/Thread call makeAnAction, it means that this one will be added to the Queue which is actually getting all its items dequeued..

I so thought about trying to create a unique link between all my cancellation tokens using CancellationTokenSource.CreateLinkedTokenSource(cancellationToken). However, even if it's a varargs logic (params), would it create the same problem?

I am just trying to achieve it in the way that it wouldn't fail, but I guess I am just having a bad approach at the moment, so I just would like to know if anyone could provide me a point of view about this encapsulation and its logic?

Feel free to give me any advice if you think that something isn't logic :)

Max

Edit 1

I then edited the code in order to follow the discussion with @NthDeveloper. I tried to add the lock system

public class CancellableSemaphoreSlim
{
    object _syncObj = new object();
    readonly Queue<CancellationTokenSource> tokens = new Queue<CancellationTokenSource>();
    readonly SemaphoreSlim ss;

    /// <summary>
    /// Initializes a new instance of the <see cref="T:Eyes.Mobile.Core.Helpers.CancellableSemaphoreSlim"/> class.
    /// </summary>
    /// <param name="initialCount">Initial count.</param>
    public CancellableSemaphoreSlim(int initialCount) { ss = new SemaphoreSlim(initialCount); }

    /// <summary>Asynchronously waits to enter the <see cref="T:System.Threading.SemaphoreSlim" />, while observing a <see cref="T:System.Threading.CancellationToken" />. </summary>
    /// <returns>A task that will complete when the semaphore has been entered. </returns>
    /// <exception cref="T:System.ObjectDisposedException">The current instance has already been disposed.</exception>
    /// <exception cref="T:System.OperationCanceledException" />
    public Task WaitAsync()
    {
        CancellationTokenSource cancellationTokenSource = new CancellationTokenSource();
        lock (_syncObj)
        {
            tokens.Enqueue(cancellationTokenSource);
        }
        return ss.WaitAsync(cancellationTokenSource.Token);
    }

    /// <summary>Asynchronously waits to enter the <see cref="T:System.Threading.SemaphoreSlim" />, while observing a <see cref="T:System.Threading.CancellationTokenSource" />. </summary>
    /// <returns>A task that will complete when the semaphore has been entered. </returns>
    /// <param name="cancellationTokenSource">The <see cref="T:System.Threading.CancellationToken" /> token to observe.</param>
    /// <exception cref="T:System.ObjectDisposedException">The current instance has already been disposed.</exception>
    /// <exception cref="T:System.OperationCanceledException">
    ///     <paramref name="cancellationTokenSource" /> was canceled. 
    /// </exception>
    public Task WaitAsync(CancellationToken cancellationToken)
    {
        CancellationTokenSource cancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
        lock (_syncObj)
        {
            tokens.Enqueue(cancellationTokenSource);
        }
        return ss.WaitAsync(cancellationTokenSource.Token);
    }

    /// <summary>
    /// Release this instance.
    /// </summary>
    /// <returns>The released semaphore return.</returns>
    public int Release() => ss.Release();

    /// <summary>
    /// Cancel all processus currently in WaitAsync() state.
    /// </summary>
    public void CancelAll()
    {
        lock (_syncObj)
        {
            while (tokens.Count > 0)
            {
                CancellationTokenSource token = tokens.Dequeue();
                if (!token.IsCancellationRequested)
                    token.Cancel();
            }
        }
    }
}

Upvotes: 1

Views: 883

Answers (2)

Peter Wishart
Peter Wishart

Reputation: 12320

I think you can simplify the code by only using a single CancellationSource, which is triggered and exchanged with a new one in CancelAll:

public sealed class CancellableSemaphoreSlim : IDisposable
{
    private CancellationTokenSource cancelSource = new();
    private readonly SemaphoreSlim semaphoreSlim;

    public CancellableSemaphoreSlim(int initialCount) => semaphoreSlim = new SemaphoreSlim(initialCount);

    public Task WaitAsync() => semaphoreSlim.WaitAsync(cancelSource.Token);

    public async Task WaitAsync(CancellationToken cancellationToken)
    {
        // This operation will cancel when either the user token or our cancelSource signal cancellation
        using var linkedSource = CancellationTokenSource.CreateLinkedTokenSource(cancelSource.Token, cancellationToken);
        await semaphoreSlim.WaitAsync(linkedSource.Token);
    }

    public int Release() => semaphoreSlim.Release();

    public void CancelAll()
    {
        using var currentCancelSource = Interlocked.Exchange(ref cancelSource, new CancellationTokenSource());
        currentCancelSource.Cancel();
    }

    public void Dispose()
    {
        cancelSource.Dispose();
        semaphoreSlim.Dispose();
    }
}

There is always going to be a race of sorts to determine whether a WaitAsync is cancelled by a call to CancelAll running at the same time.

In this version its just down to whether the old or new cancelSource.Token is grabbed in WaitAsync().

Upvotes: 3

NthDeveloper
NthDeveloper

Reputation: 1009

Sample thread safe class that protects its inner list against simultaneous changes and also prevents the class to be used after it is disposed.

public class SampleThreadSafeDisposableClass: IDisposable
{
    bool _isDisposed;
    object _syncObj = new object();

    List<object> _list = new List<object>();

    public void Add(object obj)
    {
        lock(_syncObj)
        {
            if (_isDisposed)
                return;

            _list.Add(obj);
        }
    }       

    //This method can be Dispose/Clear/CancelAll
    public void Dispose()
    {
        lock (_syncObj)
        {
            if (_isDisposed)
                return;

            _isDisposed = true;

            _list.Clear();
        }
    }
}

Hope this helps.

Upvotes: 0

Related Questions