5

I am running a thread that has a

while(isRunning) 
{ 
    blockingCollection.Take()
}

First I am setting isRunning to false. Then I call thread.Interrupt which stops the blockingCollection from waiting for new items. After that I call my Dispose Method for the class running the Thread inside the catch block.

Is that the best/correct way to do this?

Thypari
  • 801
  • 1
  • 6
  • 22
  • 5
    Anything involving `thread.Interrupt` is most likely not correct way. Instead, use `GetConsumingEnumerable` and when you are done - call `blockingColleciton.CompleteAdding()` to unblock. – Evk Nov 15 '17 at 14:29

2 Answers2

7

A BlockingCollection has a CompleteAdding() method and full support for Cancellation.

while(isRunning && ! blockingCollection.IsCompleted) 
{ 
   isRunning = blockingCollection.TryTake(out someThing, -1, cancelToken);
}

This way you can and should let the Thread end normally, always the better option.

H H
  • 263,252
  • 30
  • 330
  • 514
  • 1
    This is a decent question and a good answer; I'm not sure why it was down voted. I would like to see you explain cancellation in your answer as I'm guessing the question is completely aware of why we should do it this way. – Michael Puckett II Nov 15 '17 at 14:46
  • Instead of using `isRunning` variable it's better to use `GetConsumingEnumerable`. – Evk Nov 15 '17 at 15:28
  • Why would that be (so much) better? Besides, the question suggests there might be other logic triggering IsRunning. Not clear if the token can cover all of that. – H H Nov 15 '17 at 15:35
  • The blockingCollection is not completed when it hits 0 items. In fact that happens a lot of times during the program runtime. – Thypari Nov 16 '17 at 10:54
  • Not clear what you mean here. It should set IsCompleted when running empty _after_ CompleteAdding() was called. – H H Nov 16 '17 at 11:24
  • You are correct. Getting a CancelationException is the right way to handle it then? – Thypari Nov 16 '17 at 14:09
  • The exception is a choice (ThrowIfCancelled on the token). Makes it harder to overlook. – H H Nov 16 '17 at 14:12
  • mh... so that means when I've set it to false. And it still somehow throws the exception 3 more times. It in fact to the exception 4 times but only threw it three times because of the false argument. I have to take another look why this is happening. – Thypari Nov 16 '17 at 14:17
  • I want the blocking queue to work the remaining items or if nothing is in there, I cancel. – Thypari Nov 16 '17 at 14:26
  • I have a strange error. I get the cancelationexception even when I remove the "token.Cancel()" line... Everything is exactly like in your example. – Thypari Nov 16 '17 at 14:33
  • The code i posted was far from complete. Better ask a new question but first write a [mcve] . There are lots of details here, be complete. – H H Nov 16 '17 at 15:19
-1

Further to Henk's answer, if you'd rather have the blocking ability of Take() but need to cancel out while its waiting on an empty list, you can handle cancellation out of the loop in this way.

private void DoSomething(CancellationToken token)
{
    while (IsRunning)
    {
        MyObject next;
        try
        {
            next = _queue.Take(token);
        }
        catch(OperationCanceledException)
        {
            break; //Cancelled
        }

        // Do something with 'next'
    }

    // Cleanup

}

Then, when 'whatever' happens that requires you to kill this loop, even if the list is empty and its stuck blocking on the Take() call, just call Cancel() on your CancellationTokenSource that generated the Token.

Note - For safety, its probably also advised to catch an ObjectDisposedException that Take() can also throw. Check the documentation for details around that and why it might throw.

Iain Stanford
  • 606
  • 1
  • 6
  • 18