Network threads blocking the GUI

Question

I'm just working on C # and WPF.

I'm developing an application that should handle data transfers on the LAN.

In particular, for each transfer progress bars are displayed, green if it is a download, red if it is an upload.

Initially I had made some fictitious attempts in which I simulated transfers (the threads of the network were deactivated). So the bars were filled programmatically and I checked the performance of the GUI, everything was fine. In particular I could select the bars and make a context menu appear.

After some time, we have developed the part of the network which basically does Read and Write TCP. For each chunk (or group of chunks) a Perform Step is made so that the bar advances.

The number of chunks varies from file to file, but the number of steps to do is always 100 if there are more than 100 chunks, in this way 100 or less Perform Steps are always made for each transfer.

The problem is that when the real transfers start, everything is slowed down, when I drag the window all around and the GUI is no longer responsive I can not even click on the bars to make the context menu appear. However, the progress of the bars works.

We tried to disable the bars and make the transfers, the GUI is responsive and there are no problems.

Probably we hurt the GUI refresh from the network.

The Network has its own thread and the GUI another.

What is the best way to refresh the bars by doing it from the Network?

We currently use this, but we do not believe it is a good way, for sure it is not efficient.

For example, we take the Network Client code:

...
// Object representing a transfer (shared between GUI and Network)
Transfer t = new Transfer(...);

int bytesRead;
var buffer = new byte[chunkSize];
while ((bytesRead = file.Read(buffer, 0, buffer.Length)) > 0)
{
    if (t.Stop)
    {
        break;
    }

    nwStream.Write(buffer, 0, bytesRead);
    PBcount++;
    if (PBcount == PBchuks)
    {
        t.PerformStep(); // Make the progress bar to advance, works, but laggy
        PBcount = 0;
    }
}
...

And the code relative to the PerformStep() is a public method of the Transfer shared object:

public void PerformStep()
{
    CurrentStep = CurrentStep + 1;
    Application.Current.Dispatcher.Invoke(delegate { 
        MainWindow wnd = (MainWindow)Application.Current.MainWindow;
        wnd?.PerformStepProgressBarRefresh();
    });
}

Following is the code of the MainWindow GUI method delegated to refresh the ListBox items:

public void PerformStepProgressBarRefresh()
{
    Application.Current.Dispatcher.Invoke(delegate
    {
        TransfersXAML.Items.Refresh();
    });
}

I do not know if it can be useful, but I'll put it anyway, it's the XAML code related to the progress bars:

<ListBox x:Name="TransfersXAML"
         HorizontalContentAlignment="Stretch"
         ItemsSource="{Binding Transfers}">
    <ListBox.ItemTemplate>
        <DataTemplate>
            <ProgressBar Height="30" Minimum="0"
                         Maximum="{Binding NSteps}"
                         Value="{Binding CurrentStep}"
                         Foreground="{Binding Color}" />
        </DataTemplate>
    </ListBox.ItemTemplate>
    <ListBox.ContextMenu>
        <ContextMenu>
            <MenuItem Header="Get info" Click="GetTransferInfoClick" />
            <MenuItem Header="Cancel" Click="CancelTransferClick" />
        </ContextMenu>
    </ListBox.ContextMenu>
</ListBox>

Do you know a better and more efficient system so that the threads of the Network can update progress bars in some way?

After all, we do not know if we have done things correctly.

---

We probably did not put all the information that could be useful to better understand the problem: signal it so that we can update the question.

Update

public class Transfer
{
    public enum Type_t
    {
        upload,
        download
    }

    public int NSteps { get; set; }
    public int CurrentStep { get; set; }
    public Type_t Type { get; set; }
    public string Color { get; set; }
    public string ID { get; set; }
    public string SenderID { get; set; }
    public string ReceiverID { get; set; }

    public Transfer(string senderID, string receiverID, Type_t _type, string _color)
    {
        SenderID = senderID;
        ReceiverID = receiverID;
        NSteps = 100;
        CurrentStep = 0;
        Type = _type;
        Color = _color;
        ID = Utils.GetHashString(senderID + receiverID + Utils.GetCurrentTimestamp());
    }

    public void PerformStep()
    {
        CurrentStep = CurrentStep + 1;
   Application.Current.Dispatcher.BeginInvoke(DispatcherPriority.Background, new Action(() =>
        {
            MainWindow wnd = (MainWindow)Application.Current.MainWindow;
            wnd?.PerformStepProgressBarRefresh();
        }));
    }
}

Answer 1

You should not need to ever refresh WPF controls manually with Refresh or similar techniques. And you certainly don't need to do it here.

You correctly bound your progress bar Value property to Transfer.CurrentStep, but you didn't implement change notification to automatically notify binding about changes. For that you need to implement INotifyPropertyChanged interface on Transfer class, for example:

public class Transfer : INotifyPropertyChanged {
    private int _currentStep;
    public int CurrentStep
    {
        get { return _currentStep; }
        set
        {
            if (_currentStep != value) {
                _currentStep = value;
                OnPropertyChanged();
            }
        }
    }

    public event PropertyChangedEventHandler PropertyChanged;

    protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null) {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
    }
}

Then, remove all PerformStep() and PerformStepProgressBarRefresh - you don't need them any more, nor do you need to manually do Dispatcher.Invoke or BeginInvoke. In your network code do:

t.CurrentStep++;

That's all.