c# - SemaphoreSlim to block race condition not working, why? - Stack Overflow

admin管理员组

文章数量:1123053

I am having a race condition problem in an C# Avalonia app and I have no idea why.

The class below has 2 methods:

LoadUserData() is called when the ViewModel is created, on app start.

SaveUserData() is called before the app closes, via an event on app lifetime (something like appLifetime.Exit += viewModel.SaveUserData;). Saving should not happen if the loading didn't finish yet.

public sealed class MainWindowViewModel : ViewModelBase
{
    private static readonly SemaphoreSlim mySemaphore = new(1, 1);

    public MainWindowViewModel() => LoadUserData();

    private void LoadUserData() =>
        Task.Run(async () =>
        {
            await mySemaphore.WaitAsync();
            try
            {
                var items = await Task.WhenAll(UserDataManager.LoadUserItemsAsync());
                Dispatcher.UIThread.Post(() =>
                {
                    foreach (var item in items)
                        AddItem(item);
                });
            }
            finally
            {
                mySemaphore.Release();
            }
        });

    public void SaveUserData()
    {
        // should not save if loading didn't finish yet
        if (mySemaphore.CurrentCount == 0)
            return;

        UserDataManager.SaveUserData(this.items);
    }
}

However, there are user reports that the saving is happening while the loading didn't finish. What am I doing wrong here?

I am having a race condition problem in an C# Avalonia app and I have no idea why.

The class below has 2 methods:

LoadUserData() is called when the ViewModel is created, on app start.

SaveUserData() is called before the app closes, via an event on app lifetime (something like appLifetime.Exit += viewModel.SaveUserData;). Saving should not happen if the loading didn't finish yet.

public sealed class MainWindowViewModel : ViewModelBase
{
    private static readonly SemaphoreSlim mySemaphore = new(1, 1);

    public MainWindowViewModel() => LoadUserData();

    private void LoadUserData() =>
        Task.Run(async () =>
        {
            await mySemaphore.WaitAsync();
            try
            {
                var items = await Task.WhenAll(UserDataManager.LoadUserItemsAsync());
                Dispatcher.UIThread.Post(() =>
                {
                    foreach (var item in items)
                        AddItem(item);
                });
            }
            finally
            {
                mySemaphore.Release();
            }
        });

    public void SaveUserData()
    {
        // should not save if loading didn't finish yet
        if (mySemaphore.CurrentCount == 0)
            return;

        UserDataManager.SaveUserData(this.items);
    }
}

However, there are user reports that the saving is happening while the loading didn't finish. What am I doing wrong here?

• c#
• multithreading
• race-condition
• avaloniaui
• avalonia

Share Improve this question Follow edited 2 hours ago Theodor Zoulias 43.4k77 gold badges9999 silver badges138138 bronze badges asked 3 hours ago AlexandreAlexandre 58533 silver badges1010 bronze badges 2

• 1 The code actually has try-finally; I updated the example – Alexandre Commented 3 hours ago
• "I am having a race condition problem" - and how do you observe that? – Guru Stron Commented 3 hours ago

Add a comment  | 

2 Answers 2

Sorted by: Reset to default 1

My first observation is that you are using the CurrentCount property for control flow. This property should only be used for logging and statistics, not for controlling the execution flow. If you want to skip the SaveUserData in case the LoadUserData is currently running, you can use the Wait method passing TimeSpan.Zero as argument:

public void SaveUserData()
{
    // Should not save if loading is currently running
    if (mySemaphore.Wait(TimeSpan.Zero))
    {
        try
        {
            UserDataManager.SaveUserData(this.items);
        }
        finally
        {
            mySemaphore.Release();
        }
    }
}

The mySemaphore.Wait(TimeSpan.Zero) will wait for zero seconds, so it will not wait at all, and it will acquire the semaphore only if it can be acquired immediately.

My second observation is that you are using a mechanism that prevents concurrency, when you actually need a mechanism that tests completion. The SaveUserData() will attempt to save the data even in case the LoadUserData hasn't started yet. This in not what you want. I think that the simplest mechanism that suits your needs is the Task. So you can just replace the mySemaphore with a Task:

public sealed class MainWindowViewModel : ViewModelBase
{
    private Task _loadTask;

    public MainWindowViewModel()
    {
        _loadTask = LoadUserData();
    }

    private async Task LoadUserData()
    {
        // Switch to the ThreadPool
        var items = await Task.Run(async () =>
        {
            return await Task.WhenAll(UserDataManager.LoadUserItemsAsync());
        });

        // Back to the UI thread
        foreach (var item in items)
            AddItem(item);
    }

    public void SaveUserData()
    {
        // Should save only if loading has finished
        if (_loadTask is not null && _loadTask.IsCompleted)
            UserDataManager.SaveUserData(this.items);
    }
}

Instead of the IsCompleted you could use the IsCompletedSuccessfully, depending on what precondition you want to enforce.

The example above assumes that the SaveUserData() is invoked on the UI thread. In case you invoke it on another thread, the _loadTask should be declared as volatile (and probably the this.items as well).

Semaphore releases should only be done in finally blocks to ensure the release occurs. However, you should not need a semaphore in this instance. Just track the task and ensure it is complete before allowing the save. If you need the semaphore to ensure multiple loads of user data do not occur, just add it back in.

public sealed class MainWindowViewModel : ViewModelBase
{
    private Task? loadUserDataTask;
    private bool isLoadUserDataComplete;

    public MainWindowViewModel() => LoadUserData();

    private void LoadUserData() 
    {
        loadUserDataTask = Task.Run(async () =>
        {
            var items = await Task.WhenAll(UserDataManager.LoadUserItemsAsync());
            Dispatcher.UIThread.Post(() =>
            {
                foreach (var item in items)
                    AddItem(item);
            });
            isLoadUserDataComplete = true;
        });
    }

    public void SaveUserData()
    {
        if (!isLoadUserDataComplete || (loadUserDataTask != null && !loadUserDataTask.IsCompleted))
        {
            return;
        }

        UserDataManager.SaveUserData(this.items);
    }
}

 

本文标签： cSemaphoreSlim to block race condition not workingwhyStack Overflow