Is it a reasonable approach for lock-free design f

2019-08-29 06:44发布

问题:

This is kind of follow up to one of my earlier question here. In summary I am trying to come up with a lock free design for this scenario where I upon cancellation of task I want to call a method of third party library. In response to my question, a helpful SO participant suggested to use CancellationToken.Register but I am not sure where and how can I use that here. Below is code that I come up with. Please let me know if you see any issue with this approach or if there are any better alternatives to solve this problem.

class ProcessEmployees
    {
        private List<Employee> _Employees;
        CancellationTokenSource cs = new CancellationTokenSource();

        public  ProcessEmployees()
        {
            _Employees = new List<Employee>() 
            {
                new Employee() { ID = 1, FirstName = "John", LastName = "Doe" },
                new Employee() { ID = 2, FirstName = "Peter", LastName = "Saul" },
                new Employee() { ID = 3, FirstName = "Mike", LastName = "Sue" },
                new Employee() { ID = 4, FirstName = "Catherina", LastName = "Desoza" },
                new Employee() { ID = 5, FirstName = "Paul", LastName = "Smith" }
            };
        }

        public void StartProcessing()
        {
            try
            {
                Task[] tasks = this._Employees.AsParallel().WithCancellation(cs.Token).Select(x => this.ProcessThisEmployee(x, cs.Token)).ToArray();
                Task.WaitAll(tasks);
            }
            catch (AggregateException ae)
            {
                // error handling code
            }
            // other stuff
        }

        private async Task ProcessThisEmployee(Employee x, CancellationToken token)
        {
            ThirdPartyLibrary library = new ThirdPartyLibrary();
            if (token.IsCancellationRequested)
                token.ThrowIfCancellationRequested();
            await Task.Factory.StartNew(() => library.SomeAPI(x) );
            if (token.IsCancellationRequested)
            {
                library.Clean();
                token.ThrowIfCancellationRequested();
            }
        }
    }

回答1:

Your process code can be simplified to the following (here is how you use Register)

private void ProcessThisEmployee(Employee x, CancellationToken token)
{
    ThirdPartyLibrary library = new ThirdPartyLibrary();
    token.ThrowIfCancellationRequested();
    using(token.Register(() => library.Clean())
    {
        library.SomeAPI(x);
    }
    token.ThrowIfCancellationRequested(); //Not sure why you cancel here in your original example.
}

If the token is canceled while within the scope of the using statement it will call library.Clean() if it is called afterwards it does not call the function. I also got rid of your Task.Run, there is no reason to waste the extra thread for what you are doing. Lastly I got rid of the extra if (token.IsCancellationRequested) checks, ThrowIfCancellationRequested() has that if check inside of itself, you don't need to check beforehand.

Also, because in my simplification you are not returning tasks anymore your StartProcessing code becomes

    public void StartProcessing()
    {
        try
        {
            this._Employees.AsParallel().WithCancellation(cs.Token).ForAll(x => this.ProcessThisEmployee(x, cs.Token));
        }
        catch (AggregateException ae)
        {
            // error handling code
        }
        // other stuff
    }

using ForAll( instead of Select(