-1

I get back false from BlockingCollection's TryTake method, when the BlockingCollection is empty, although the expected behavior is to block until the collection fills up.

Note that the collection is not upper bounded (which should affect the TryAdd not the TryTake) and that the Timeout set for the add operation had not passed.

Here's my wrapper around the BlockingCollection object:

    public T TryTake(int timeoutMiliseconds)
    {
        var result = default(T);

        if (!_collection.TryTake(out result, timeoutMiliseconds))
        {
            throw new InvalidOperationException("Unable to get item from collection.");
        }

        return result;
    }

Any ideas what can cause this ?

I've implemented the Producer-Consumer pattern, based on this article: Multithread processing of the SqlDataReader - Producer/Consumer design pattern

ilans
  • 2,537
  • 1
  • 28
  • 29
  • What does the *message* included with the exception say? – Damien_The_Unbeliever May 23 '18 at 08:52
  • I've added it to the post – ilans May 23 '18 at 08:54
  • 2
    `TryTake` does not throw this exception because collection it empty, it throws it for other reasons (described in documentation of this method) – Evk May 23 '18 at 08:56
  • 2
    For the benefit of others, the method being called in the code above is NOT `BlockingCollection.TryTake`. It is a method of a custom class that, internally, calls `BlockingCollection.TryTake` and throws the specified exception if an item cannot be taken within the specified timeout period. Follow the link provided to see that code. I did and realised that my answer wasn't applicable. – jmcilhinney May 23 '18 at 09:06
  • 2
    As @jmcilhinney mentions this exception is literally thrown in the `TryTake` method you copy pasted from your link. At least read code you are copy pasting, or use a debugger which would point you right at the code throwing that exception. – Evk May 23 '18 at 09:16
  • The exception is documented well, it tells you that the code that adds items to the collection has a bug. It is not doing it in a thread-safe way. We can't see it. – Hans Passant May 23 '18 at 09:30
  • You're right guys, I got confused between the original collection TryTake and the wrapping TryTake. But the fact is that the TryTake returns false instead of block on the TryTake. I guess that's because the `CompleteAdding` is marked true like in this SO post: https://stackoverflow.com/questions/21195224/when-can-blockingcollectiont-trytake-return-false. I'm verifying it. – ilans May 23 '18 at 11:22

1 Answers1

-1

I finally realized that as this SO post demonstrates, I got false out of TryTake cause the collection had been emptied, and also marked by me as completed (by calling CompleteAdding() method upon the collection).

So in order to solve it, I've added a condition on the completed flag.

One can argue that TryTake would never return false while the completed flag is false. I'm not sure that's always right, so I prefer to throw an error in this case.

    public T TryTake(int timeoutMiliseconds)
    {
        var result = default(T);

        if (!_collection.TryTake(out result, timeoutMiliseconds) 
            && !_collection.IsAddingCompleted)
        {
            throw new InvalidOperationException("Unable to get item from collection.");
        }

        return result;
    }

NOTE that it's crucial to put the TryTake call first and the IsAddingCompleted second. Otherwise the first condition might pass and you'd block on the TryTake right after.

ilans
  • 2,537
  • 1
  • 28
  • 29
  • Could there be a race condition here, TryTake can return false, and then the collection is marked as completed, before you check IsAddingCompleted? – Lasse V. Karlsen May 23 '18 at 12:45
  • Could be. But a Consumers use this method to take out items inserted by the producer, so in the case you've described, another consumer would have taken the item before the current consumer in your scenario.. which is ok. – ilans May 23 '18 at 14:30
  • Except that you will get back `default(T)`? – Lasse V. Karlsen May 23 '18 at 17:45
  • Yes. The user of this function should be aware of the possibility to get a default(T) which is in practice null, in most cases. – ilans May 24 '18 at 05:38