Is it safe to put TryDequeue in a while loop?

问题:

I have not used concurrent queue before.

Is it OK to use TryDequeue as below, in a while loop? Could this not get stuck forever?

var cq = new ConcurrentQueue<string>();
cq.Enqueue("test");

string retValue;

while(!cq.TryDequeue(out retValue))
{
    // Maybe sleep?
}

//Do rest of code

回答1:

Yes it is safe as per the documentation, but it is not a recommended design.

It might get "Stuck forever" if the queue was empty at the first call TryDequeue, and if no other thread pushes data in the queue after that point (you could break the while after N attempts or after a timeout, though).

ConcurrentQueue offers an IsEmpty member to check if there are items in the Queue. It is much more efficient to check it than to loop over a TryDequeue call (particularly if the queue is generally empty)

What you might want to do is :

while(cq.IsEmpty())
{
    // Maybe sleep / wait / ...
}

if(cq.TryDequeue(out retValue))
{
...
}

EDIT: If this last call returns false: another of your threads dequeued the item. If you don't have other threads, this is safe, if you do, you should use while(TryDequeue)

回答2:

It's safe in the sense that the loop won't actually end until there is an item it has pulled out, and that it will eventually end if the queue has an item to be taken out. If the queue is emptied by another thread and no more items are added then of course the loop will not end.

Beyond all of that, what you have is a busy loop. This should virtually always be avoided. Either you end up constantly polling the queue asking for more items, wasting CPU time and effort tin the process, or you end up sleeping and therefore not actually using the item in the queue as soon as it is added (and even then, still wasting some time/effort on context switches just to poll the queue).

What you should be doing instead, if you find yourself in the position of wanting to "wait until there is an item for me to take" is use a BlockingCollection. It is specifically designed to wrap various types of concurrent collections and block until there is an item available to take. It allows you to change your code to queue.Take() and have it be easier to write, semantically stating what you're doing, be clearly correct, noticeably more effective, and completely safe.