3

I have a task, which executed async, in part of this task add items in UI run via Dispatcher.BeginInvoke where i update a ObservebleCollection. For thread safe access to collection, i use a semaphoreSlim, but as request to Collection proceed in UI thread and Dispatcher.BeginInvoke also work in UI thread, i receive a dead lock.

    private readonly ObservebleCollection<String> parameters = new ObservebleCollection<String>();
    private readonly SemaphoreSlim semaphore = new SemaphoreSlim(0, 1);
    //Called from UI
    public ObservebleCollection<String> Parameters
    {
        get
        {
          semaphore.Wait();
          var result = this.parameters;
          semaphore.Release();

          return result;
        }
    }

    public async Task Operation()
    {
            await semaphore.WaitAsync();
            List<String> stored = new List<String>();
            foreach (var parameter in currentRobot.GetParametersProvider().GetParameters())
            {
                stored.Add(parameter.PropertyName);
            }

            //Can't do add all items in UI at once, because it's take a long time, and ui started lag
            foreach (var model in stored)
            {
                await UIDispatcher.BeginInvoke(new Action(() =>
                {
                    this.parameters.Add(model);
                }), System.Windows.Threading.DispatcherPriority.Background);
            }
            semaphore.Release();
        }

And how i received a dead lock: When i click a button in my program, Operation executed. When i click a another button, program try access to Parameters property. And i received a dead lock =D

Problem: in async operation i fill a observeblecollection via Dispatcher.BeginInvoke for each item separately, because if i add all items at once using Dispatcher, UI will lag. So i need a synchronization method for access to a Parameters property, which will wait until Operation ends.

Panagiotis Kanavos
  • 120,703
  • 13
  • 188
  • 236
l1pton17
  • 444
  • 4
  • 16
  • `await` ensures code will run on the correct thread, so what's the point of using BeginInvoke? Also why do you try to lock `Parameters` when all modifications happen in the same method, *in the same thread*? – Panagiotis Kanavos Dec 15 '14 at 08:13
  • 2
    I can't make sense from your code. You could better explain what you're trying to achieve in plain english. That will be much better. – Sriram Sakthivel Dec 15 '14 at 08:18
  • Can't say I understand what the code does either. I don't see a single point where asynchronous operations are used, nor anything going on in the background. What is the *real* problem you are trying to solve here? – Panagiotis Kanavos Dec 15 '14 at 08:20
  • I add item using Dispatcher with background priority, because if add all items at once, ui will lag. – l1pton17 Dec 15 '14 at 08:40
  • Why are you doing this? Why do you need to use a `Semaphore` at all? You're updating UI elements, they can only be accessed from the UI thread. – Yuval Itzchakov Dec 15 '14 at 08:41
  • As i say, i'm updating UI elements with background priority. So a some operation with higher priority can try access to Parameters property, which still updated with background priority. – l1pton17 Dec 15 '14 at 08:42
  • Your code isn't doing anything in the background. What you wrote as a comment (and makes sense) is that your UI freezes when adding too many items at the same time. That's a WPF issue that is solved with WPF techniques like raising NotifyPropertyChanged or NotifyCollectionChanged once you finish adding items, not once for each item – Panagiotis Kanavos Dec 15 '14 at 08:49
  • If i do like you say, ui still freeze when adding. – l1pton17 Dec 15 '14 at 09:11
  • What did you try? Where is the code? If you modify an ObservableCollection one item at a time, then yes, the UI freezes, that's the problem. Even if you raise the event afterwards, you've already raised it a dozen times already. If the UI freezes even when using a `List` (with only a single PropertyChanged event raised), then there is a problem with the rest of the UI. Without the code though, it's impossible to guess – Panagiotis Kanavos Dec 15 '14 at 09:15
  • Let i repeat a question: i need a synchronization method to access a Collection in ui thread with higher priority, when update process add each item via Dispatcher.BeginInvoke with background priority. And nobody can't access to Collection until update process finished. – l1pton17 Dec 15 '14 at 09:18
  • I use a collectionview with source a list. I fill a list in ui thread, it works fast. After that i call a CollectionView.Refresh(), it still lag. – l1pton17 Dec 15 '14 at 09:20

1 Answers1

3

await ensures the code after it will run in the original Synchronization context, in this case, in the UI thread. This makes BeginInvoke unnecessary as the code already runs on the correct thread.

The result is that you are trying to acquire two locks on the same object from the same thread, resulting in deadlock.

If you want thread-safe access to a collection of objects, avoid manually creating locks and use a thread-safe collection like ConcurrentQueue or ConcurrentDictionary.

Apart from that, I can't say I understand what the code tries to achieve as it does nothing in the background or asynchronously. It could easily be a simple method that copies parameters from one collection to another and it would still be thread safe if written properly. You could just write:

var _parameters=new ConcurrentQueue<string>();

....

public void CopyParameters()
{
     foreach (var parameter in currentRobot.GetParametersProvider().GetParameters())
     {
            _parameters.Enqueue(parameter.PropertyName);
     }
}

If you use databinding on the Parameters property, just raise PropertyChanged after you add all entries

What is the real problem you are trying to solve?

UPDATE

It seems the real problem is the UI freezes if you try to add too many items at a time. This isn't a threading problem, it's a WPF problem. There are various solutions, all of which involve raising PropertyChanged only after you finish adding all properties.

If you don't need the old values, just create a list with the new values, replace the old values then raise the PropertyChanged event, eg:

private ObservebleCollection<String> _parameters = new ObservebleCollection<String>();

public ObservebleCollection<String> Parameters 
{
    get 
    { 
        return _parameters;
    }
    private set 
    {
        _parameters=value;
        PropertyChanged("Parameters");
    }

    public void CopyParameters()
    {
        var newParameters=currentRobot.GetParametersProvider()
                                      .GetParameters()
                                      .Select(p=>p.PropertyName);
        Parameters=new ObservableCollection<string>(newParameters);
    }

Unless you have code that modified Parameters one item at a time though, you could easily swap ObservableCollection for any other collection type, even a string[] array.

Another option is to subclass ObservableCollection to add support for AddRange, as shown in this SO question

Community
  • 1
  • 1
Panagiotis Kanavos
  • 120,703
  • 13
  • 188
  • 236
  • Problem: in async operation i fill a observeblecollection via Dispatcher.BeginInvoke for each item separately, because if i add all items at once using Dispatcher, UI will lag. – l1pton17 Dec 15 '14 at 08:36
  • 1
    So your *real* problem is that you are adding a lot of items to the collection and the UI can't update fast enough. That's a WPF problem. The solution to this is to *not* use an ObservableCollection but raise the NotifyPropertyChanged event after adding all items. Or, you could use. What you tried to do would still run on the UI thread, resulting in the same issues. – Panagiotis Kanavos Dec 15 '14 at 08:46