Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: ChannelReader<T>.TryPeek #31362

Closed
stephentoub opened this issue Nov 1, 2019 · 4 comments · Fixed by #53436
Closed

Proposal: ChannelReader<T>.TryPeek #31362

stephentoub opened this issue Nov 1, 2019 · 4 comments · Fixed by #53436
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Threading.Channels
Milestone

Comments

@stephentoub
Copy link
Member

stephentoub commented Nov 1, 2019

Per comment at https://github.com/dotnet/corefx/issues/41819#issuecomment-542819724, we should consider adding:

namespace System.Threading.Channels
{
    public abstract partial class ChannelReader<T> // existing class
    {
        public virtual bool TryPeek([MaybeNullWhen(false)] out T item) // new virtual method
        {
            item = default!;
            return false;
        }
        ...
    }
}

We'd need to confirm, but I believe it should be possible to override this with a valid implementation on all of the built-in channel implementations.

NOTE: Now that we have a {Can}Count property, it's quite possible for Count to return non-zero value but TryPeek to not be overridden and return false.

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 5.0 milestone Feb 1, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@stephentoub stephentoub added api-ready-for-review and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation untriaged New issue has not been triaged by the area owner labels Feb 25, 2020
@terrajobst
Copy link
Member

terrajobst commented Jun 12, 2020

Video

  • We should add a virtual CanPeek that returns false
  • We'll want more feedback so we should punt this to .NET 6.
namespace System.Threading.Channels
{
    public abstract partial class ChannelReader<T>
    {
        public virtual bool CanPeek => false;

        public virtual bool TryPeek([MaybeNullWhen(false)] out T item)
        {
            item = default!;
            return false;
        }
    }
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review labels Jun 12, 2020
@terrajobst terrajobst modified the milestones: 5.0, 6.0 Jun 12, 2020
@KevinCathcart
Copy link
Contributor

Was the conclusion that this should return false in the not CanPeek scenario? I just watched the video, and was left the feeling that throwing was still desired if not CanPeek. Which is not what the approved API summary is showing. But perhaps I missed something.

@Dean-NC
Copy link

Dean-NC commented May 7, 2021

I need this. I'm using channels for the first time, and I assumed it would be available on the reader since ConcurrentQueue<> has TryPeek. I have an API that receives "file changed" requests, where the caller lets me know 1 or more files from a CRM service changed and provides download URLs. The API controller queues them up and gives an acknowledgment back to the caller. A hosted BackgroundService in my API reads the queue and uploads them to a document archive web service.

Before removing an item from the queue, I'd like to know I uploaded the files for that request successfully, then de-queue. Since the BackgroundService is the only thing in my API that reads the queue, I'm not worried about other threads processing the same item.

I was going to use ConcurrentQueue but then I remembered Channels, and decided to use them.

@stephentoub stephentoub self-assigned this May 28, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 28, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 7, 2021
@Dean-NC
Copy link

Dean-NC commented Jun 7, 2021

@stephentoub Thanks for doing this!! I'm looking forward to this feature in 6.0!

@ghost ghost locked as resolved and limited conversation to collaborators Jul 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Threading.Channels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants