Friday, January 19, 2024

Blocking the UI thread might not be so bad

I can't believe it's been so long since I last posted. We've been very busy here at work.

We have a standard client/server architecture and when the server is doing things, the client is not responsive because the call to the server is made on the UI thread. This means the user cannot move or resize the client while it's waiting for the server to do its thing. Many people think this is bad.

I have been following the UK postal service scandal where hundreds of post office masters were prosecuted for theft and fraud because of errors in the new accounting software they were required to use.

https://www.theguardian.com/uk-news/2024/jan/09/how-the-post-offices-horizon-system-failed-a-technical-breakdown

One thing in the article that caught my attention was that clicking the [Enter] button multiple times while the software appeared to be frozen caused the transaction to be sent multiple times.

One, named the “Dalmellington Bug”, after the village in Scotland where a post office operator first fell prey to it, would see the screen freeze as the user was attempting to confirm receipt of cash. Each time the user pressed “enter” on the frozen screen, it would silently update the record. In Dalmellington, that bug created a £24,000 discrepancy, which the Post Office tried to hold the post office operator responsible for.

Our software does not have that problem, because we are communicating with the server on the UI thread. Is there an alternative?

I saw this project that explains how to implement a task to execute code off the UI thread. I converted it to be MVVM compliant and noticed you can still click the button even while the code from the first click was executing. This is similar to the Dalmellington bug.

Here's the WPF project with the code run on the UI thread. The project is written in C# using .Net 8 and is called Tasks.

<Window x:Class="Tasks.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        xmlns:local="clr-namespace:Tasks"
        mc:Ignorable="d"
        DataContext="{Binding RelativeSource={RelativeSource Self}}"
        Title="MainWindow" Height="450" Width="800">
    <Window.Resources>
        <RoutedCommand x:Key="RunCommand"/>
    </Window.Resources>
    <Window.CommandBindings>
        <CommandBinding Command="{StaticResource RunCommand}" Executed="Run_Executed"/>
    </Window.CommandBindings>
    <Grid>
        <Button Content="The Button" Command="{StaticResource RunCommand}"/>
    </Grid>
</Window>

using System.Windows;
namespace Tasks
{
    public partial class MainWindow : Window
    {
        public MainWindow()
        {
            InitializeComponent();
        }

        private void Run_Executed(object sender, System.Windows.Input.ExecutedRoutedEventArgs e)
        {
            for (int i = 0; i != 100; ++i)
            {
                Thread.Sleep(100);
            }
        }
    }
}



Clicking the button runs a task on the UI thread. The user cannot move, resize, or close the application until the task has completed. Button clicks while the task is still running are ignored.

When I implemented a Task to run the code off the UI thread I added a progress monitor and the project looks like this. Note progress bars don't like null values so I wrote a converter.

<Window x:Class="Tasks.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        xmlns:local="clr-namespace:Tasks"
        mc:Ignorable="d"
        DataContext="{Binding RelativeSource={RelativeSource Self}}"
        Title="MainWindow" Height="450" Width="800">
    <Window.Resources>
        <local:NullConverter x:Key="NullConverter"/>
        <RoutedCommand x:Key="RunCommand"/>
    </Window.Resources>
    <Window.CommandBindings>
        <CommandBinding Command="{StaticResource RunCommand}" Executed="Run_Executed"/>
    </Window.CommandBindings>
    <Grid>
        <Button Content="The Button" Command="{StaticResource RunCommand}"/>
        <ProgressBar Value="{Binding ProgressValue, Converter={StaticResource NullConverter}}"
VerticalAlignment="Top" HorizontalAlignment="Stretch" Height="10">
            <ProgressBar.Style>
                <Style TargetType="ProgressBar">
                    <Setter Property="Visibility" Value="Visible"/>
                    <Style.Triggers>
                        <DataTrigger Binding="{Binding ProgressValue}" Value="{x:Null}">
                            <Setter Property="Visibility" Value="Hidden"/>
                        </DataTrigger>
                    </Style.Triggers>
                </Style>
            </ProgressBar.Style>
        </ProgressBar>
    </Grid>
</Window>


using System.ComponentModel;
using System.Runtime.CompilerServices;
using System.Windows;

namespace Tasks
{
    public partial class MainWindow : Window, INotifyPropertyChanged
    {
        private int? _ProgressValue = null;
        public int? ProgressValue
        {
            get => _ProgressValue;
            set { SetProperty(ref _ProgressValue, value); }
        }

        public MainWindow()
        {
            InitializeComponent();
        }

        private async void Run_Executed(object sender, System.Windows.Input.ExecutedRoutedEventArgs e)
        {
            IProgress<int> progress = new Progress<int>(value => ProgressValue = value);
            await Task.Run(() =>
            {
                for (int i = 0; i != 100; ++i)
                {
                    progress.Report(i);
                    Thread.Sleep(100);
                }
            });
            ProgressValue = null;
        }

        public event PropertyChangedEventHandler? PropertyChanged;
        public void SetProperty<T>(ref T storage, T value, [CallerMemberName] string name = "")
        {
            if (!Object.Equals(storage, value))
            {
                storage = value;
                if (PropertyChanged != null)
                    PropertyChanged(this, new PropertyChangedEventArgs(name));
            }
        }
    }
}

The NullConverter goes in a class library and looks like this.

using System.Globalization;
using System.Windows.Data;

namespace Tasks
{
    class NullConverter : IValueConverter
    {
        public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
        {
           return (value == null) ? 0 : System.Convert.ToInt32(value);
        }

        public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
        {
            throw new NotImplementedException();
        }
    }
}


Because we are executing the code in a task, the UI still responds to the user. They can move, resize, and close the application. But they can also still click the button. Clicking the button while the task is executing causes the task to execute again. If the task was posting a new transaction, it would be posted multiple times.

A solution is to bind the IsEnabled property of the window or root element to the ProgressValue property via a new converter. That's why I made ProgressValue nullable.

Here's the new converter and the modified XAML.

    class IsNullConverter : IValueConverter
    {
        public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
        {
            return (value == null);
        }

        public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
        {
            throw new NotImplementedException();
        }
    }

<Window x:Class="Tasks.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        xmlns:local="clr-namespace:Tasks"
        mc:Ignorable="d"
        DataContext="{Binding RelativeSource={RelativeSource Self}}"
        Title="MainWindow" Height="450" Width="800">
    <Window.Resources>
        <local:NullConverter x:Key="NullConverter"/>
        <local:IsNullConverter x:Key="IsNullConverter"/>
        <RoutedCommand x:Key="RunCommand"/>
    </Window.Resources>
    <Window.CommandBindings>
        <CommandBinding Command="{StaticResource RunCommand}" Executed="Run_Executed"/>
    </Window.CommandBindings>
    <Grid IsEnabled="{Binding ProgressValue, Converter={StaticResource IsNullConverter}}">
        <Button Content="The Button" Command="{StaticResource RunCommand}"/>
        <ProgressBar Value="{Binding ProgressValue, Converter={StaticResource NullConverter}}"
VerticalAlignment="Top" HorizontalAlignment="Stretch" Height="10">
            <ProgressBar.Style>
                <Style TargetType="ProgressBar">
                    <Setter Property="Visibility" Value="Visible"/>
                    <Style.Triggers>
                        <DataTrigger Binding="{Binding ProgressValue}" Value="{x:Null}">
                            <Setter Property="Visibility" Value="Hidden"/>
                        </DataTrigger>
                    </Style.Triggers>
                </Style>
            </ProgressBar.Style>
        </ProgressBar>
    </Grid>
</Window>

This causes all the controls on the page to be disabled while the task is executing (while ProgressValue is not null). If you run the application now, you can still move and resize the client while the task is executing but button clicks are ignored.

I could have bound the Window's IsEnabled property, but the XAML to do that is a little more complex and binding the root element is just as good.

So blocking the UI thread while performing background tasks has its benefits, but there is a fairly easy way to safely perform tasks on non-UI threads, especially if it is implemented in a base class.

No comments:

Post a Comment