2

I am trying to create a way to queue up Tasks to run, so I have tried to implement it using a BlockingCollection. The problem I find is whenever I try to add the Task, the Task executes. Sample code as below:

private void button1_Click(object sender, EventArgs e)
{
    textBox2.Clear();
    for (int i = 0; i < 10; i++)
    _processCollection.Add(BigTask(i));
}

static BlockingCollection<Task> _processCollection = new BlockingCollection<Task>();
Thread ConsumerThread = new Thread(LaunchConsumer);

private static async void LaunchConsumer()
{
    while (true)
    {
        var processTask = _processCollection.Take();
        await Task.Run(() => processTask);
    }
}

async Task BigTask(int i)
{
    await Task.Delay(5000);
    textBox2.AppendText($"Text{i}\n");
}

What seems to happen in debug is all the tasks seem to run as they are added into the blocking collection. I tried switching the blocking collection to use Action, but that just leads to nothing happening. As below (only changes shown):

private void button1_Click(object sender, EventArgs e)
{
    textBox2.Clear();
    for (int i = 0; i < 10; i++)
    {
        int iC = i;
        _processCollection.Add(async () => await BigTask(iC));
    }
}

static BlockingCollection<Action> _processCollection = new BlockingCollection<Action>();
Thread ConsumerThread = new Thread(LaunchConsumer);

private static async void LaunchConsumer()
{
    while (true)
    {
        var processTask = _processCollection.Take();
        await Task.Run(processTask);
    }
}

I feel like I have made some small error somewhere, because it feels like this should work. I have tried to find someone doing something similar but have had no luck, which makes me think maybe my concept is flawed so feel free to suggest an alternative.

1 Answers1

2

_processCollection.Add(BigTask(i)); doesn't work because this calls BigTask(i) immediately, and when that is called, the work starts.

You were on the right track by wrapping this in a separate BigTask launcher, but by using Action, you don't provide your LaunchConsumer with any means to track the progress. await Task.Run(processTask) will continue pretty much immediately with the next task. You need to use Func<Task> to avoid that.

The reason you don't see any results is likely unrelated. Now that you manage to launch the task from your newly created thread, the call to textBox2.AppendText is no longer done from the UI thread. That's not supported. Only the UI thread can access UI objects. You can use textBox2.Invoke to pass an action back to the UI thread, and that action can then call AppendText.

Tested working code:

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();
        ConsumerThread.Start();
    }

    private void button1_Click(object sender, EventArgs e)
    {
        textBox2.Clear();
        foreach (var i in Enumerable.Range(0, 10))
            _processCollection.Add(() => BigTask(i));
    }

    static BlockingCollection<Func<Task>> _processCollection = new BlockingCollection<Func<Task>>();
    Thread ConsumerThread = new Thread(LaunchConsumer);

    private static async void LaunchConsumer()
    {
        while (true)
        {
            var processTask = _processCollection.Take();
            await Task.Run(processTask);
        }
    }

    async Task BigTask(int i)
    {
        await Task.Delay(5000);
        textBox2.Invoke(new Action(() => textBox2.AppendText($"Text{i}\n")));
    }
}

That said, BlockingCollection is not really the best collection type to use here. It dedicates one thread to pretty much nothing but waiting. Also, Task.Run when you're already in a background thread can admittedly sometimes be useful, but doesn't add anything here. What to do instead depends on your needs. Whether all tasks are known beforehand makes a difference. Whether you may want multiple consumers makes a difference. Other things I haven't thought of may also make a difference.

  • Hi, Could you clarify what you mean about using `Func`. I have tried using it in the blocking collection and had no change in result. Is the syntax still the same to how I had it when using `Action`. – Mayura Vivekananda Apr 21 '16 at 07:37
  • In regards to the not seeing results, my original code example has non UI stuff so I am reasonably sure its not executing. I put in a invoke in this example anyway, and still don't get a result showing. – Mayura Vivekananda Apr 21 '16 at 07:37
  • In this case, you can just change `Action` to `Func` without any other changes. `async () => { }` can convert to either an `async void` delegate or an `async Task` delegate, and the compiler will determine which based on what the caller expects. I'll try to amend my answer to show something that I'll have been able to test later. –  Apr 21 '16 at 07:43
  • That sounds like what I have tried. Would be interested to see your example code. – Mayura Vivekananda Apr 22 '16 at 05:42
  • @MayuraVivekananda I've now included code that I've been able to test. I also had to change the `for` loop to `foreach` because `for` only uses a single variable, continually updates it, so all the big tasks would see `i` with the same value (`10`). –  Apr 22 '16 at 08:27
  • 1
    Hah, I realised the error looking at my code. I wasn't starting my consumer thread (doh!). Thanks a lot for posting the full answer. Out of interest what do you recommend over a `BlockingCollection`. I am after something that allows me to implement a FIFO collection with `x` number of consumers. – Mayura Vivekananda Apr 22 '16 at 08:57
  • @MayuraVivekananda Heh, yeah, code that isn't started won't work. :) I think a `ConcurrentQueue` is almost good enough for that. You just need some way to allow for non-blocking waits (to allow the background threads to do other work) while the queue is empty, and that should be doable by creating a wrapper around `ConcurrentQueue` that combines it with `TaskCompletionSource` objects that get created whenever the queue is emptied. –  Apr 22 '16 at 09:12