0

I've got a process that takes a long time so I want to break it out into Threads. My threading approach works fine with Parallel.ForEach, only I want to inform a user how many of the variable number of items we've processed so far.

Here's a sample of what I'm doing.

namespace TestingThreading
{
    public partial class MainWindow : Window, INotifyPropertyChanged
    {
        private int _idCounter;
        public int IdCounter
        {
            get { return _idCounter; }
            set
            {
                if (value != _idCounter)
                {
                    _idCounter = value;
                    OnPropertyChanged("IdCounter");
                }
            }
        }
        public event PropertyChangedEventHandler PropertyChanged;
        protected virtual void OnPropertyChanged(string name)
        {
            var handler = System.Threading.Interlocked.CompareExchange(ref PropertyChanged, null, null);
            if (handler != null)
            {
                handler(this, new PropertyChangedEventArgs(name));
            }
        }

        public MainWindow()
        {
            InitializeComponent();
            Counter.SetBinding(ContentProperty, new Binding("IdCounter"));
            DataContext = this;
            IdCounter = 0;
        }

        //random wait as a stand in for a variable length task
        private void GetWait()
        {
            Random random = new Random();
            int w = random.Next(3, 15);
            System.Threading.Thread.Sleep(100 * w);
        }

        private async void CreateClientsButton_Click(object sender, RoutedEventArgs e)
        {   
            //setup my a list of strings to iterate through 
            List<String> DeviceList = new List<string>();
            for (int i = 0; i < 150; i++)
            {
                string ThisClient = "Fox" + i;
                DeviceList.Add(ThisClient);

            }

            var myTask = Task.Run(() =>
            {
                Parallel.ForEach(DeviceList, new ParallelOptions { MaxDegreeOfParallelism = 15 }, device =>
                {
                    GetWait();
                    IdCounter++;
                    // both below give compiler errors
                    //System.Threading.Interlocked.Add(ref IdCounter, 1);
                    //var c = Interlocked.Increment(ref DeviceCounter);
                });
            });

            await myTask;

        }

    }
}

The binding to the UI works, but I'm pretty sure I'm not actually incrementing the variable as many times as I should be. For instance, this will run 150 iterations but after multiple attempts, I've never seen my counter end higher than 146, which leasds me to believe there's a race condition when two threads try to update the variable at the same time.

It's not the end of the world, but I'd love to do this 'the right way', and my research leads me to Interlock.Increment or Interlock.Add, but when I try to increment my variable with either of these, I get this error:

A property or indexer may not be passed as an out or ref parameter

FoxDeploy
  • 12,569
  • 2
  • 33
  • 48
  • 1
    Yeah this is really not thread safe at all. and trying to interlock after you have called a setter then raised a property changed event is iffy to say the least – TheGeneral May 02 '19 at 03:28
  • 2
    So your goal is to just do a bunch of tasks in parallel, and update the ui with the current completed count? without blocking the ui – TheGeneral May 02 '19 at 03:33
  • Use local variable for Interlocked.Increment, once the job is completed assign it to your property. – Dmytro Mukalov May 02 '19 at 03:36
  • @thegeneral that is exactly what I want to do! Is using interlocked like this not the way I should go about it? – FoxDeploy May 02 '19 at 12:44
  • @DmytroMukalov would you mind showing an example of how to do that? I've looked at a ton of similar questions on StackOverflow and can't find one that works in my code. – FoxDeploy May 02 '19 at 13:16
  • 1
    Something like [this](https://dotnetfiddle.net/cRRdcQ) – Dmytro Mukalov May 02 '19 at 14:27

1 Answers1

1

I strongly recommend using IProgress<T> for updating the UI. It's unusual to use a "stateful" progress (i.e., "increment") instead of a "stateless" progress (i.e., "Item 13 has completed."), but it is doable.

Note that Progress<T> takes care of synchronizing with the UI thread, so it solves the race condition for you.

var progress = new Progress<int>(_ => IdCounter++) as IProgress<int>;
var myTask = Task.Run(() =>
{
  Parallel.ForEach(DeviceList, new ParallelOptions { MaxDegreeOfParallelism = 15 }, device =>
  {
    GetWait();
    progress.Report(0);
  });
});
Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810