Task.Factory.StartNew() return thread to thread pool

Question

In this implementation Task.Factory.StartNew will never return thread to thread pool because it contains while(true) with Thread.Sleep. Is it right? How to check queue that it has task that need to be done?

namespace ConsoleApplication1
{
class Program
{
    static void Main(string[] args)
    {
        Helper h = new Helper();
        PlayWithQueue s = new PlayWithQueue();
        s.AddToQueueForExecution(() => { h.Indicate(1); });
        s.AddToQueueForExecution(() => { h.Indicate(2); });
        s.AddToQueueForExecution(() => { h.Indicate(3); });
        s.AddToQueueForExecution(() => { h.Indicate(4); });
        s.AddToQueueForExecution(() => { h.Indicate(5); });
        s.AddToQueueForExecution(() => { h.Indicate(6); });

        Console.ReadKey();
    }
}
public class PlayWithQueue
{
    private readonly ConcurrentQueue<Action> queue = new ConcurrentQueue<Action>();

    public PlayWithQueue()
    {
        var task = Task.Factory.StartNew(ThreadProc);
    }

    public void AddToQueueForExecution(Action action)
    {
        queue.Enqueue(action);
    }
    private void ThreadProc()
    {
        while (true)
        {
            Action item;
            bool isSuccessfull = false;
            isSuccessfull = queue.TryDequeue(out item);
            if (isSuccessfull)
            {
                item();
            }
            System.Threading.Thread.Sleep(100);
        }
    }
}
public class Helper
{
    public void Indicate(int number)
    {
        Random rnd = new Random();
        int timeDelay = rnd.Next(1000, 5000);
        Console.WriteLine("Start" + number.ToString());
        System.Threading.Thread.Sleep(timeDelay);
        Console.WriteLine("End" + number.ToString() + " " + timeDelay.ToString());
    }
}
}

Yup, you steal one ThreadPool thread forever. What do you mean by `How to check queue that it has task that need to be done?`? — FCin, Apr 06 '18 at 19:11
If you are going to have a really long running thread like this, don't use the thread-pool, just create a real `Thread`. Using the thread-pool for something like this just ends up introducing extra complexity. — Bradley Uffner, Apr 06 '18 at 19:19
If you really want to do this correctly, look up the "Producer / Consumer Pattern". — Bradley Uffner, Apr 06 '18 at 19:24
You can avoid holding thread with await Task.Delay, but there are better ways to do that still. — Evk, Apr 06 '18 at 19:29
Consider using `BlockingCollection` rather than `ConcurrentQueue` to void the need for `Thread.Sleep`. — mjwills, Apr 06 '18 at 21:22

score -1 · Answer 1 · answered Apr 06 '18 at 19:22

-1

Change your ThreadProc method to this. Loop until queue has an item.

private void ThreadProc()
    {
        Action item;
        while (queue.TryDequeue(out item))
        {
            item();
            System.Threading.Thread.Sleep(100);
        }
    }

answered Apr 06 '18 at 19:22

Ayaz

2,111
4
13
16

This doesn't make any difference at all - still the thread is blocked forever. – Ventsyslav Raikov Apr 06 '18 at 19:26
1

queue.TryDequeue method will populate the item into out parameter (item) and remove the item from queue. So in each iteration number of tasks in queue will be decremented. Once all the tasks are dequeued, queue.Dequeue method will return false, hence it will come out of while loop and thread will be returned to thread pool. – Ayaz Apr 06 '18 at 19:26
@Bond, Can you please explain how doesn't it make any difference? once all the items are dequeued, while loop will be terminated and execution will come out of while so thread will be released. – Ayaz Apr 06 '18 at 19:30
yes you're right. I meant it didn't make difference from threading point. But your solution is no correct - in the original code you have a loop that will never end - it will consume all items added to the queue. You just consume all items that are `currently` in the queue and end. – Ventsyslav Raikov Apr 06 '18 at 19:35

Ventsyslav Raikov · Answer 2 · 2018-04-06T19:39:45.990

-1

Ok, haven't written any C# in a while but here is how you may not block a thread

private async void ThreadProc()
    {
        while (true)
        {
            Action item;
            bool isSuccessfull = false;
            isSuccessfull = queue.TryDequeue(out item);
            if (isSuccessfull)
            {
                item();
            }
            await Task.Delay(300);
        }
    }

But maybe if I were to implement this I would just run the action directly in AddToQueueForExecution instead of queuing it on another thread... If you want this to be thread safe i.e. no 2 actions to run at the same time - just use a lock.

edited Apr 06 '18 at 19:39

answered Apr 06 '18 at 19:33

Ventsyslav Raikov

6,882
1
25
28

1

I would at least consider making this return a `Task` instead of void, and have it take a `CancellationToken` that can be used to cleanly break out of the loop if needed. – Bradley Uffner Apr 06 '18 at 19:37

Task.Factory.StartNew() return thread to thread pool

2 Answers2