Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 10, 2025

BlockingCollection.TakeFromAny and TryTakeFromAny ignore pre-cancelled tokens when items are immediately available. The fast path bypasses cancellation checks, allowing operations to succeed even when cancellation was requested before the call.

Changes

  • BlockingCollection.cs: Added externalCancellationToken.ThrowIfCancellationRequested() at the start of TryTakeFromAnyCore before the fast path loop
  • BlockingCollectionCancellationTests.cs: Added test covering TakeFromAny and TryTakeFromAny variants with pre-cancelled tokens and available items

Example

var bc1 = new BlockingCollection<int>();
var bc2 = new BlockingCollection<int>();
bc1.Add(1);
bc2.Add(2);

var cts = new CancellationTokenSource();
cts.Cancel();

// Previously: would successfully take item 1 or 2
// Now: throws OperationCanceledException immediately
BlockingCollection<int>.TakeFromAny(new[] { bc1, bc2 }, out int item, cts.Token);

This aligns with standard cancellation semantics where a cancelled token throws immediately regardless of operation feasibility.

Original prompt

This section details on the original issue you should resolve

<issue_title>BlockingCollection.TakeFromAny suggestion</issue_title>
<issue_description>### Description

BlockingCollecction.TakeFromAny do not check state of cancellationToken (3rd argument).

inside of this method :

    // Check if the collection is not completed, and potentially has at least one element by checking the semaphore count
    if (!collections[i].IsCompleted && collections[i]._occupiedNodes.CurrentCount > 0 && **collections[i].TryTake(out item))**

in call of TryTake(out item)) do not used cancellationToken from method call and inside of TryTake(out item)) used call with empty CancellationToken:

      public bool TryTake([MaybeNullWhen(false)] out T item)
        {
            return TryTake(out item, 0, **CancellationToken.None**);
        }

I propose use here :
// Check if the collection is not completed, and potentially has at least one element by checking the semaphore count
if (!collections[i].IsCompleted && collections[i]._occupiedNodes.CurrentCount > 0 && collections[i].TryTake(out item, cancellationToken ))

Reproduction Steps

    internal class Program
    {
        static void Main(string[] args)
        {
            int value = 0;
            CancellationTokenSource cts = new CancellationTokenSource(5000);
            BlockingCollection<int> collection = new BlockingCollection<int>(new ConcurrentStack<int>(), 10);
            BlockingCollection<int> priority = new BlockingCollection<int>(new ConcurrentStack<int>(), 3);
            BlockingCollection<int>[] list = new BlockingCollection<int>[] { priority, collection };
            var producer = Task.Run(() =>
            {
                try
                {
                    while (value < 20)
                    {
                        collection.Add(value++, cts.Token);
                        Console.WriteLine("+++added.");
                        Task.Delay(500).Wait();
                    }
                }
                catch(OperationCanceledException)
                {
                    Console.WriteLine("Canceled");
                }
                
            });
            var producer2 = Task.Run(() =>
            {
                Task.Delay(2500).Wait();
                try
                {
                    while (value < 20)
                    {
                        priority.Add(value++ + 100, cts.Token);
                        Console.WriteLine("+++added pr.");
                        Task.Delay(500).Wait();
                    }
                }
                catch (OperationCanceledException)
                {
                    Console.WriteLine("Canceled");
                }

            });
            var consumer = Task.Run(() =>
            {
                try
                {
                    while (true)
                    {
                        //cts.Token.ThrowIfCancellationRequested();   recomment here for immediate cancel
                        _ = BlockingCollection<int>.TakeFromAny(list, out int val, cts.Token);
                        Console.WriteLine("---taked " + val.ToString());
                        Task.Delay(1000).Wait();
                    }
                }
                catch (OperationCanceledException)
                {
                    Console.WriteLine("Canceled");
                }
            });
            Console.ReadLine();
    }
}

Expected behavior

Take operation canceled if cancellationToken canceled and any of collection not empty

Actual behavior

Now Take operation do not canceled if cancellationToken canceled and any of collection not empty

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response</issue_description>

Comments on the Issue (you are @copilot in this section)

@ I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one [area label](https://github.com/dotnet/runtime/blob/master/docs/area-owners.md). @ Tagging subscribers to this area: @dotnet/area-system-collections See info in [area-owners.md](https://github.com/dotnet/runtime/blob/main/docs/area-owners.md) if you want to be subscribed.
Issue Details

Description

BlockingCollecction.TakeFromAny do not check state of cancellationToken (3rd argument).

inside of this method :

// Check if the collection is not completed, and potentially has at least one element by checking the semaphore count
if (!collections[i].IsCompleted && collections[i]._occupiedNodes.CurrentCount > 0 && **collections[i].TryTake(out item))**...

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
@stephentoub stephentoub marked this pull request as ready for review December 11, 2025 20:39
Copilot AI review requested due to automatic review settings December 11, 2025 20:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug where BlockingCollection.TakeFromAny and TryTakeFromAny methods ignore pre-cancelled tokens when items are immediately available. The fast path optimization bypassed cancellation checks, allowing operations to succeed even when cancellation was requested before the call.

Key Changes:

  • Added early cancellation token check in TryTakeFromAnyCore before the fast path loop executes
  • Added comprehensive test coverage for TakeFromAny and TryTakeFromAny variants with pre-cancelled tokens and available items

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
BlockingCollection.cs Added externalCancellationToken.ThrowIfCancellationRequested() at the start of TryTakeFromAnyCore to honor pre-cancelled tokens before attempting the fast path
BlockingCollectionCancellationTests.cs Added test ExternalCancel_TakeFromAny_PreCanceled_WithAvailableItems covering all three method variants (TakeFromAny, TryTakeFromAny with infinite timeout, TryTakeFromAny with finite timeout) to ensure they throw when given a pre-cancelled token

@stephentoub stephentoub enabled auto-merge (squash) December 12, 2025 05:06
@stephentoub stephentoub merged commit 3828390 into main Dec 12, 2025
83 of 85 checks passed
@eiriktsarpalis eiriktsarpalis deleted the copilot/fix-blockingcollection-takefromany branch December 12, 2025 14:29
@github-actions github-actions bot locked and limited conversation to collaborators Jan 12, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BlockingCollection.TakeFromAny suggestion

3 participants