Awaiting a queue not working as expected.

[RossMetacraft]

Hello! I came across this library while searching for a way to run tasks in a serial fashion to avoid any concurrency issues in my application. It looks like it will work perfectly. In your Examples and Programming Models Wiki page, you demonstrate a very simple and elegant way of achieving serialization of tasks by simply awaiting the queue at the start of each task method. I tried doing this in a simple proof-of-concept application and it does not appear to be working as I expect. I'm either doing something wrong, or this library isn't actually designed to do what I thought. I'm hoping you can set me straight.

In my simple test app, I have a window with two buttons. One button adds random integers to a list. The other button iterates over the list, printing the values to the debug console. It sleeps for 500 ms between prints. I'm hoping to be able to populate the list with a few integers, then iterate the list, and try to add another integer to the list while the list is being iterated, and avoid the usual InvalidOperationException due to the collection being modified during iteration.

Here is the window's code-behind file:

using System.Windows;
using Dispatch;

namespace SerialQueueTest;

public partial class MainWindow : Window
{
	private readonly SerialQueue mSerialQueue = new();
	private readonly List<int> mList = [];

	public MainWindow()
	{
		InitializeComponent();
	}

	private async void Iterate_Click(object sender, RoutedEventArgs e)
	{
		await IterateInts();
	}

	private async void Add_Click(object sender, RoutedEventArgs e)
	{
		await AddInt();
	}

	private async Task IterateInts()
	{
		await mSerialQueue;
		foreach (int i in mList) {
			System.Diagnostics.Debug.WriteLine($"iterating: {i}");
			await Task.Delay(500);
		}
	}

	private async Task AddInt()
	{
		await mSerialQueue;
		int i = Random.Shared.Next();
		System.Diagnostics.Debug.WriteLine($"adding {i}");
		mList.Add(i);
	}
}

To run the test, I launch the app, click the "Add" button 5 times to populate the list with 5 random integers. I then click the "Iterate" button, and while it is iterating, I click the "Add" button again to attempt to add a 6th integer to the list. This causes the usual InvalidOperationException with the message "Collection was modified; enumeration operation may not execute."

Am I missing something, or does the lib not behave the way I'm expecting here?

Thank you!

[borland]

Hey @RossMetacraft, just letting you know I've just seen this issue. I'll need to find some time to reproduce the problem and form a response.

To help with that, would you mind providing a bit more context around your application? Is it using WPF or Windows Forms or is this inconsequential?

[RossMetacraft]

Hi @borland, the test app that I posted above to illustrate the issue is a WPF app, but my actual application is a background service running within an ASP.net core application.

Thanks for taking a look.

[borland]

I haven't been able to run your example, but I've had a think and I'm pretty sure I know what's happening here.

The serial queue is working as designed, but perhaps not as you expect; threading and concurrency is full of pitfalls like this.

I don't know your level of familiarity with async/await in C#, so I'll start from the bottom; My apologies if I'm explaining things you already know.

The serial queue offers the same kind of model as the WPF dispatcher thread does, or the nodejs main eventloop. Each queue makes the following guarantees:

• Operations you enqueue onto the queue will always execute in the order they were enqueued
• Only one operation will execute at a time.

But what exactly constitutes an "operation" is not always obvious. Let's take a look at your IterateInts function

private async Task IterateInts()
{
    await mSerialQueue;
    foreach (int i in mList) {
        System.Diagnostics.Debug.WriteLine($"iterating: {i}");
        await Task.Delay(500);
    }
}

The way await works in C# is that the compiler slices up your function into pieces, using await as the boundary.
The way the "slicing" works is basically by spreading a switch statement across your function, with cases at each await.
This snippet on sharplab lets you see it in all the gory detail if you're interested.

Anyway, a useful mental model is more like this:

private async Task IterateInts()
{
    switch(state)
    {
        case 0:
            mSerialQueue.RunCallback(/* re-enter the function but with state set to 1 */);
            return;
        case 1:
            foreach (int i in mList) {
                System.Diagnostics.Debug.WriteLine($"iterating: {i}");
                Task.Delay(500).ThenRunCallback(/* re-enter the function but with state set to 2 */);
                return;
        case 2:
                // tail end of the loop.
                // That's right, the switch case is in the middle of the loop, so it goes round again
            }    
    }
}

What we can see here is that the function returns and then gets re-entered after the asynchronous thing.
What looks like one "operation" logically (it's one function), is actually 3 distinct operations, with gaps in between.

During the gaps (e.g while waiting for Task.Delay), there is no code running.
The SerialQueue is free and can run other things, such as your button click handler, without violating any of the rules.

So your bug is:

• IterateInts does await mSerialQueue, so it is executing on the queue
• foreach loop creates an Enumerator object behind the scenes, which starts stepping through the list
• We hit Task.Delay, so our function returns, ready to be resumed later
  • The Enumerator is still open, hanging out in the background waiting for the resume
  • The queue is free.
• You click the button, which does await mSerialQueue, so also executing on the queue
• AddInt modifies mList on the queue.
• Later, the Task.Delay finishes. Because the serial queue is a synchronization context, the "continuation" is also on the queue
• It resumes, goes round the loop and the foreach invokes MoveNext on the Enumerator.
  • This sees that the thing it was enumerating over has changed, kaboom

Or, put in a different way: Your foreach loop needs the list not to be modified while it is walking the list.
You can't stick an await anywhere that you can't handle other things coming in and pulling the rug out from you. This is the same underlying reason behind why you can't await inside a lock statement.

Two fixes:

1. Don't await in the loop; It will then be one single operation and work as you expect
2. Take a copy of mList and do the foreach over that instead

I hope that helps, thanks!

[RossMetacraft]

Hi again @borland, thank you so much for the detailed explanation. I did already have a fairly good understanding of how async/await works and how it creates a state machine for executing your async method as a series of steps, but it's good to see it explained as you did.

If I'm understanding you correctly, when you await the queue, you are essentially saying "wait until the queue is free". Put another way, awaiting the queue is short hand for wrapping the rest of the method in an action and passing that action to DispatchAsync(). Is that right?

In my test app, what I was expecting was that the serial queue would guarantee that the IterateInts() method would complete fully before any other method (such as AddInt()) was executed. But it sounds like you're saying that would only be true if IterateInts() and AddInt() were synchronous methods with no awaits in them. As soon as something is awaited (such as the call to Task.Delay(500) then the queue is no longer guaranteeing serial execution and it is free to move on to other queued tasks.

If all of the above is correct, then it makes me wonder why the examples in the main Readme show asynchronous stuff happening while execution is "on the queue". Specifically, this bit:

SerialQueue m_queue = new SerialQueue();

// imagine this is a WPF or winforms app
public void Button_Click()
{
    // here we are on the UI main thread
    var result = await DoBackgroundProcessing();

    // and we are still on the UI thread because 'await DoBackgroundProcessing' captured the sync context.
    MyTextBox.Text = result;
}

private async Task<string> DoBackgroundProcessing()
{
    // at this point we are still on the UI main thread
        
    await m_queue;

    // now we are OFF the main UI thread and onto the serial queue (behind the scenes we're on a threadpool thread)

    var response = await SendNetworkRequest();

    // still on the serial queue

    return response;
}

In this example, wouldn't the awaited call to SendNetworkRequest() cause the queue to be freed up and potentially move on to another task, before DoBackgroundProcessing() completes, thus making it no longer a serial queue?

I think I'm probably expecting too much of the concept of a "serial queue". I'm expecting it to run the tasks in order, never more than one at a time. In other words, each task fully completes before the next begins. It sounds like it really only guarantees that no two tasks will run at the same time on two separate threads. Two tasks may still run at the same time, in the sense that task B may be started before task A completes, if task A is async and contains an await call. In this case, both tasks are "running" at the same time because both have been started, and neither has completed, but one of them is "paused" (awaiting some async operation) while the other is actually executing.

How am I doing? 😄