CODEWITHSUNDEEP
c#wpfmvvmobservablecollection
duffmanseven
duffmanseven

Reputation: 175

WPF MVVM updating ObservableCollection from Threadpool

I was reading that my implementation of updating an observable collection from a background worker thread (using Task.Run()) may not be thread safe.

In my application I have a ListView bound to an observable collection with the selected item two-way bound to my viewmodel as well. While the application is idle I have this ListView update it's values every 60 seconds. This update can be triggered manually so I designed it as an async function as it requests values from a Database. When my control is loaded and in an idle state, a loop function is run under Task.Run().

public ObservableCollection<WorkFlowTask> WorkFlowTasks { get; set; }
private WorkFlowTask _SelectedTask; 
        public WorkFlowTask SelectedTask
        {
            get
            {
                return _SelectedTask;
            }
            set
            {
                _SelectedTask = value;
                OnPropertyChanged("SelectedTask");
            }
        }

private void SynchTaskList()
        {
            TokenSourceTaskList = new CancellationTokenSource();
            CancelTokenTaskList = TokenSourceTaskList.Token;

            Task.Run(() => UpdateTaskList());
        }

UpdateTaskList is defined as follows :

private async void UpdateTaskList ()
        {
            while (!CancelTokenTaskList.IsCancellationRequested)
            {
                List<WorkFlowTask> tasks = await Connector.GetWorkFlowTasksByEmpID(Employee.EmployeeID);
                if(SelectedTask != null)
                {
                    //Preserve currently selected item through collection update
                    SelectedTask = tasks.Where(x => x.TransWorkFlowDetailID == SelectedTask.TransWorkFlowDetailID).FirstOrDefault();
                }                
                
                var newWorkFlowTasks = new ObservableCollection<WorkFlowTask>(tasks.OrderBy(x => x.DueDate));
                
                if (!WorkFlowTasks.SequenceEqual(newWorkFlowTasks))
                {
                    WorkFlowTasks = newWorkFlowTasks;
                }
                OnPropertyChanged("WorkFlowTasks");
                await Task.Delay(60000);
            }            
        }

Functionally, this is working as I expect but I fear this may not be thread safe. Is this only working because I'm replacing the collection with a new object instead of modifying it? Does the update to the collection need to be Invoked? Should I wrap a lock around the SelectedItem property changes as well?

Any insight or advice would be greatly appreciated.

Upvotes: 0

Views: 196

Answers (1)

NoConnection
NoConnection

Reputation: 842

First of all, you do not need to invoke the changes to SelectedTask and WorkflowTasks, as you are consuming them via databinding, which is asynchronous by itself. Normally accessing the collection will be fine.

Since you wrote it is possible to manually invoke the update mechanism of the WorkflowTasks, it probably is a good idea to wrap them inside a lock statement. Otherwise you could run into issues when two parallel tasks are modifying the SelectedTask at the same time.

I would place the lock right after fetching the tasklist. That way multiple tasks can fetch the list simultaneously (which i guess is a time-consuming task), but will write the result to your properties sequentially.

private async void UpdateTaskList()
    {
        while (!CancelTokenTaskList.IsCancellationRequested)
        {
            List<WorkFlowTask> tasks = await Connector.GetWorkFlowTasksByEmpID(Employee.EmployeeID);

            lock (this.taskListLock)
            {
                if (SelectedTask != null)
                {
                    //Preserve currently selected item through collection update
                    SelectedTask = tasks.Where(x => x.TransWorkFlowDetailID == SelectedTask.TransWorkFlowDetailID).FirstOrDefault();
                }

                var newWorkFlowTasks = new ObservableCollection<WorkFlowTask>(tasks.OrderBy(x => x.DueDate));

                if (!WorkFlowTasks.SequenceEqual(newWorkFlowTasks))
                {
                    WorkFlowTasks = newWorkFlowTasks;
                }
                OnPropertyChanged("WorkFlowTasks");
            }

            await Task.Delay(60000);
        }
    }

Upvotes: 1

Related Questions